diff --git a/.travis.yml b/.travis.yml index 20089f8..16d442a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -43,3 +43,7 @@ matrix: gemfile: gemfiles/rails_3.2_stable.gemfile - rvm: jruby-1.7 gemfile: gemfiles/rails_5.0.gemfile + +notifications: + slack: + secure: gpltVWntdKz0nSE6A5UvuX4qbN35uW51nsW+Ojgqm8Qsv8K240/NlZRYutFHr7GnJTe0rEEP2Oy3ZBnBtZKFn13RlTEAU/FCAxebr4H24rr29Ypwwp5xjiSE4MuoBEnroo4lw6ka3LsJnrY2PKRMiLJGsS0WsEPY4x8NUG/vyY8= diff --git a/CHANGELOG.md b/CHANGELOG.md index d4ba7e9..3ee4421 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # CHANGELOG +## unreleased + + * fix: permissible states not respecting guard parameters(see [issue #388](https://github.com/aasm/aasm/issues/388)) with [pull-request #389] + +## 4.11.0 + + * support `logger` configuration (see [issue #370](https://github.com/aasm/aasm/pull/370) for details, thanks to [@HoyaBoya](https://github.com/HoyaBoya)) + * support configuration to let bang transitions fail if object is invalid (see [issue #366](https://github.com/aasm/aasm/pull/366) and [issue #262](https://github.com/aasm/aasm/issues/262) for details, thanks to [@Wildebeest](https://github.com/Wildebeest)) + + ## 4.10.1 * fix: suppress warnings when using ActiveRecord enums feature (see [issue #346](https://github.com/aasm/aasm/pull/346) for details, thanks to [@110y](https://github.com/110y), and [issue #353](https://github.com/aasm/aasm/pull/353) for details, thanks to [@nathanstitt](https://github.com/nathanstitt)) @@ -323,4 +333,3 @@ * supporting i18n * supporting regular expressions for hash values and strings - diff --git a/README.md b/README.md index 7289919..cb08fca 100644 --- a/README.md +++ b/README.md @@ -132,7 +132,7 @@ class Job ... end - def notify_somebody(user) + def notify_somebody ... end @@ -418,6 +418,28 @@ SimpleMultipleExample.aasm(:work).states *Final note*: Support for multiple state machines per class is a pretty new feature (since version `4.3`), so please bear with us in case it doesn't work as expected. +### Auto-generated Status Constants + +AASM automatically [generates constants](https://github.com/aasm/aasm/pull/60) +for each status so you don't have to explicitly define them. + +```ruby +class Foo + include AASM + + aasm do + state :initialized + state :calculated + state :finalized + end +end + +> Foo::STATE_INITIALIZED +#=> :initialized +> Foo::STATE_CALCULATED +#=> :calculated +``` + ### Extending AASM AASM allows you to easily extend `AASM::Base` for your own application purposes. @@ -522,7 +544,7 @@ Saving includes running all validations on the `Job` class, and returns `true` i successful or `false` if errors occur. Exceptions are not raised. If you want make sure the state gets saved without running validations (and -thereby maybe persisting aninvalid object state), simply tell AASM to skip the +thereby maybe persisting an invalid object state), simply tell AASM to skip the validations. Be aware that when skipping validations, only the state column will be updated in the database (just like ActiveRecord `update_column` is working). @@ -869,6 +891,7 @@ end AASM supports query methods for states and events Given the following `Job` class: + ```ruby class Job include AASM @@ -889,7 +912,7 @@ class Job transitions :from => [:running, :cleaning], :to => :sleeping end end - + def cleaning_needed? false end @@ -934,9 +957,29 @@ job.aasm.events(:reject => :sleep).map(&:name) # list states for select Job.aasm.states_for_select => [["Sleeping", "sleeping"], ["Running", "running"], ["Cleaning", "cleaning"]] + +# show permitted states with guard parameter +job.aasm.states({:permitted => true}, guard_parameter).map(&:name) ``` +### Warning output + +Warnings are by default printed to `STDERR`. If you want to log those warnings to another output, +use + +```ruby +class Job + include AASM + + aasm :logger => Rails.logger do + ... + end +end +``` + +Be aware though, that this is not yet released. It will be part of _AASM_ version `4.11.0`. + ### RubyMotion support Now supports [CodeDataQuery](https://github.com/infinitered/cdq.git) ! @@ -1037,6 +1080,7 @@ Feel free to * [Scott Barron](https://github.com/rubyist) (2006–2009, original author) * [Travis Tilley](https://github.com/ttilley) (2009–2011) * [Thorsten Böttger](http://github.com/alto) (since 2011) +* [Anil Maurya](http://github.com/anilmaurya) (since 2016) ## Contributing ## diff --git a/aasm.gemspec b/aasm.gemspec index 8dcb14f..5e49fc6 100644 --- a/aasm.gemspec +++ b/aasm.gemspec @@ -5,8 +5,8 @@ require "aasm/version" Gem::Specification.new do |s| s.name = "aasm" s.version = AASM::VERSION - s.authors = ["Scott Barron", "Travis Tilley", "Thorsten Boettger"] - s.email = %q{scott@elitists.net, ttilley@gmail.com, aasm@mt7.de} + s.authors = ["Thorsten Boettger", "Anil Maurya"] + s.email = %q{aasm@mt7.de, anilmaurya8dec@gmail.com} s.homepage = %q{https://github.com/aasm/aasm} s.summary = %q{State machine mixin for Ruby objects} s.description = %q{AASM is a continuation of the acts-as-state-machine rails plugin, built for plain Ruby objects.} diff --git a/gemfiles/rails_4.0_mongo_mapper.gemfile b/gemfiles/rails_4.0_mongo_mapper.gemfile index ab15ea2..707fd6b 100644 --- a/gemfiles/rails_4.0_mongo_mapper.gemfile +++ b/gemfiles/rails_4.0_mongo_mapper.gemfile @@ -1,7 +1,6 @@ source "https://rubygems.org" gem "sqlite3", :platforms => :ruby -gem "coveralls" gem 'rubysl', :platforms => :rbx gem 'rubinius-developer_tools', :platforms => :rbx gem "jruby-openssl", :platforms => :jruby diff --git a/gemfiles/rails_4.2_mongo_mapper.gemfile b/gemfiles/rails_4.2_mongo_mapper.gemfile index 62a8550..c113b26 100644 --- a/gemfiles/rails_4.2_mongo_mapper.gemfile +++ b/gemfiles/rails_4.2_mongo_mapper.gemfile @@ -1,7 +1,6 @@ source "https://rubygems.org" gem "sqlite3", :platforms => :ruby -gem "coveralls" gem 'rubysl', :platforms => :rbx gem 'rubinius-developer_tools', :platforms => :rbx gem "jruby-openssl", :platforms => :jruby diff --git a/lib/aasm/base.rb b/lib/aasm/base.rb index baf0d1b..80e77f1 100644 --- a/lib/aasm/base.rb +++ b/lib/aasm/base.rb @@ -1,3 +1,5 @@ +require 'logger' + module AASM class Base @@ -21,6 +23,9 @@ module AASM # don't store any new state if the model is invalid (in ActiveRecord) configure :skip_validation_on_save, false + # raise if the model is invalid (in ActiveRecord) + configure :whiny_persistence, false + # use requires_new for nested transactions (in ActiveRecord) configure :requires_new_transaction, true @@ -40,6 +45,9 @@ module AASM # Set to true to namespace reader methods and constants configure :namespace, false + # Configure a logger, with default being a Logger to STDERR + configure :logger, Logger.new(STDERR) + # make sure to raise an error if no_direct_assignment is enabled # and attribute is directly assigned though aasm_name = @name @@ -202,7 +210,7 @@ module AASM klass.defined_enums.values.any?{ |methods| methods.keys{| enum | enum + '?' == method_name } }) - warn "#{klass.name}: overriding method '#{method_name}'!" + @state_machine.config.logger.warn "#{klass.name}: overriding method '#{method_name}'!" end klass.send(:define_method, method_name, method_definition) diff --git a/lib/aasm/configuration.rb b/lib/aasm/configuration.rb index 5d9b04e..832cd08 100644 --- a/lib/aasm/configuration.rb +++ b/lib/aasm/configuration.rb @@ -9,7 +9,10 @@ module AASM # for all persistence layers: create named scopes for each state attr_accessor :create_scopes - # for ActiveRecord: don't store any new state if the model is invalid + # for ActiveRecord: when the model is invalid, true -> raise, false -> return false + attr_accessor :whiny_persistence + + # for ActiveRecord: store the new state even if the model is invalid and return true attr_accessor :skip_validation_on_save # for ActiveRecord: use requires_new for nested transactions? @@ -28,5 +31,8 @@ module AASM # namespace reader methods and constants attr_accessor :namespace + + # Configure a logger, with default being a Logger to STDERR + attr_accessor :logger end end diff --git a/lib/aasm/instance_base.rb b/lib/aasm/instance_base.rb index 91d8d67..5ded621 100644 --- a/lib/aasm/instance_base.rb +++ b/lib/aasm/instance_base.rb @@ -34,9 +34,9 @@ module AASM AASM::Localizer.new.human_state_name(@instance.class, state_object_for_name(current_state)) end - def states(options={}) + def states(options={}, *args) if options.has_key?(:permitted) - selected_events = events(:permitted => options[:permitted]) + selected_events = events({:permitted => options[:permitted]}, *args) # An array of arrays. Each inner array represents the transitions that # transition from the current state for an event event_transitions = selected_events.map {|e| e.transitions_from_state(current_state) } @@ -46,10 +46,10 @@ module AASM return nil if transitions.empty? # Return the :to state of the first transition that is allowed (or not) or nil - if options[:permitted] - transition = transitions.find { |t| t.allowed?(@instance) } + if options[:permitted] + transition = transitions.find { |t| t.allowed?(@instance, *args) } else - transition = transitions.find { |t| !t.allowed?(@instance) } + transition = transitions.find { |t| !t.allowed?(@instance, *args) } end transition ? transition.to : nil end.flatten.compact.uniq @@ -61,7 +61,7 @@ module AASM end end - def events(options={}) + def events(options={}, *args) state = options[:state] || current_state events = @instance.class.aasm(@name).events.select {|e| e.transitions_from_state?(state) } @@ -72,9 +72,9 @@ module AASM # filters the results of events_for_current_state so that only those that # are really currently possible (given transition guards) are shown. if options[:permitted] - events.select! { |e| @instance.send("may_#{e.name}?") } + events.select! { |e| @instance.send("may_#{e.name}?", *args) } else - events.select! { |e| !@instance.send("may_#{e.name}?") } + events.select! { |e| !@instance.send("may_#{e.name}?", *args) } end end diff --git a/lib/aasm/persistence/active_record_persistence.rb b/lib/aasm/persistence/active_record_persistence.rb index 659542e..07bce5e 100644 --- a/lib/aasm/persistence/active_record_persistence.rb +++ b/lib/aasm/persistence/active_record_persistence.rb @@ -78,7 +78,12 @@ module AASM self.save end - success ? true : aasm_rollback(name, old_value) + unless success + aasm_rollback(name, old_value) + raise ActiveRecord::RecordInvalid.new(self) if aasm_whiny_persistence(name) + end + + success end # Writes state to the state column, but does not persist it to the database @@ -132,6 +137,10 @@ module AASM AASM::StateMachineStore.fetch(self.class, true).machine(state_machine_name).config.skip_validation_on_save end + def aasm_whiny_persistence(state_machine_name) + AASM::StateMachineStore.fetch(self.class, true).machine(state_machine_name).config.whiny_persistence + end + def aasm_write_attribute(state, name=:default) write_attribute(self.class.aasm(name).attribute_name, aasm_raw_attribute_value(state, name)) end diff --git a/lib/aasm/version.rb b/lib/aasm/version.rb index 494ce74..e77a67c 100644 --- a/lib/aasm/version.rb +++ b/lib/aasm/version.rb @@ -1,3 +1,3 @@ module AASM - VERSION = "4.10.1" + VERSION = "4.11.0" end diff --git a/spec/database.rb b/spec/database.rb index 1c542f5..b004487 100644 --- a/spec/database.rb +++ b/spec/database.rb @@ -17,7 +17,7 @@ ActiveRecord::Migration.suppress_messages do t.string "right" end - %w(validators multiple_validators).each do |table_name| + %w(validators multiple_validators workers invalid_persistors multiple_invalid_persistors silent_persistors multiple_silent_persistors).each do |table_name| ActiveRecord::Migration.create_table table_name, :force => true do |t| t.string "name" t.string "status" @@ -32,20 +32,6 @@ ActiveRecord::Migration.suppress_messages do end end - ActiveRecord::Migration.create_table "workers", :force => true do |t| - t.string "name" - t.string "status" - end - - ActiveRecord::Migration.create_table "invalid_persistors", :force => true do |t| - t.string "name" - t.string "status" - end - ActiveRecord::Migration.create_table "multiple_invalid_persistors", :force => true do |t| - t.string "name" - t.string "status" - end - ActiveRecord::Migration.create_table "fathers", :force => true do |t| t.string "aasm_state" t.string "type" diff --git a/spec/models/guard_with_params.rb b/spec/models/guard_with_params.rb new file mode 100644 index 0000000..a657bd4 --- /dev/null +++ b/spec/models/guard_with_params.rb @@ -0,0 +1,24 @@ +class GuardWithParams + include AASM + aasm do + state :new, :reviewed, :finalized + + event :mark_as_reviewed do + transitions :from => :new, :to => :reviewed, :guards => [:user_is_manager?] + end + end + + def user_is_manager?(user) + ok = false + if user.has_role? :manager + ok = true + end + return ok + end +end + +class GuardParamsClass + def has_role?(role) + true + end +end diff --git a/spec/models/guard_with_params_multiple.rb b/spec/models/guard_with_params_multiple.rb new file mode 100644 index 0000000..86532c0 --- /dev/null +++ b/spec/models/guard_with_params_multiple.rb @@ -0,0 +1,18 @@ +class GuardWithParamsMultiple + include AASM + aasm(:left) do + state :new, :reviewed, :finalized + + event :mark_as_reviewed do + transitions :from => :new, :to => :reviewed, :guards => [:user_is_manager?] + end + end + + def user_is_manager?(user) + ok = false + if user.has_role? :manager + ok = true + end + return ok + end +end diff --git a/spec/models/silent_persistor.rb b/spec/models/silent_persistor.rb new file mode 100644 index 0000000..944660d --- /dev/null +++ b/spec/models/silent_persistor.rb @@ -0,0 +1,31 @@ +require 'active_record' + +class SilentPersistor < ActiveRecord::Base + include AASM + aasm :column => :status, :whiny_persistence => false do + state :sleeping, :initial => true + state :running + event :run do + transitions :to => :running, :from => :sleeping + end + event :sleep do + transitions :to => :sleeping, :from => :running + end + end + validates_presence_of :name +end + +class MultipleSilentPersistor < ActiveRecord::Base + include AASM + aasm :left, :column => :status, :whiny_persistence => false do + state :sleeping, :initial => true + state :running + event :run do + transitions :to => :running, :from => :sleeping + end + event :sleep do + transitions :to => :sleeping, :from => :running + end + end + validates_presence_of :name +end diff --git a/spec/models/validator.rb b/spec/models/validator.rb index 0475dfb..67f3c9d 100644 --- a/spec/models/validator.rb +++ b/spec/models/validator.rb @@ -10,7 +10,7 @@ class Validator < ActiveRecord::Base include AASM - aasm :column => :status do + aasm :column => :status, :whiny_persistence => true do before_all_transactions :before_all_transactions after_all_transactions :after_all_transactions @@ -84,7 +84,7 @@ end class MultipleValidator < ActiveRecord::Base include AASM - aasm :left, :column => :status do + aasm :left, :column => :status, :whiny_persistence => true do state :sleeping, :initial => true state :awake state :running diff --git a/spec/unit/guard_with_params_multiple_spec.rb b/spec/unit/guard_with_params_multiple_spec.rb new file mode 100644 index 0000000..abb7302 --- /dev/null +++ b/spec/unit/guard_with_params_multiple_spec.rb @@ -0,0 +1,10 @@ +require 'spec_helper' + +describe "guards with params" do + let(:guard) { GuardWithParamsMultiple.new } + let(:user) {GuardParamsClass.new} + + it "list permitted states" do + expect(guard.aasm(:left).states({:permitted => true}, user).map(&:name)).to eql [:reviewed] + end +end diff --git a/spec/unit/guard_with_params_spec.rb b/spec/unit/guard_with_params_spec.rb new file mode 100644 index 0000000..a91b534 --- /dev/null +++ b/spec/unit/guard_with_params_spec.rb @@ -0,0 +1,10 @@ +require 'spec_helper' + +describe "guards with params" do + let(:guard) { GuardWithParams.new } + let(:user) {GuardParamsClass.new} + + it "list permitted states" do + expect(guard.aasm.states({:permitted => true}, user).map(&:name)).to eql [:reviewed] + end +end diff --git a/spec/unit/override_warning_spec.rb b/spec/unit/override_warning_spec.rb index eb71964..3826732 100644 --- a/spec/unit/override_warning_spec.rb +++ b/spec/unit/override_warning_spec.rb @@ -28,42 +28,59 @@ describe 'warns when overrides a method' do end describe 'state' do - class Base - def valid?; end + let(:base_klass) do + Class.new do + def valid?; end + end end - it do - expect { Base.send :include, Clumsy }. - to output(/Base: overriding method 'valid\?'!/).to_stderr + + subject { base_klass.send :include, Clumsy } + + it 'should log to warn' do + expect_any_instance_of(Logger).to receive(:warn).with(": overriding method 'valid?'!") + subject end end describe 'enum' do - class EnumBase - def valid?; end + let(:enum_base_klass) do + Class.new do + def valid?; end + end end - it "dosn't warn when overriding an enum" do - expect { EnumBase.send :include, WithEnumBase }. - not_to output(/EnumBase: overriding method 'valid\?'!/).to_stderr + + subject { enum_base_klass.send :include, WithEnumBase } + + it 'should not log to warn' do + expect_any_instance_of(Logger).to receive(:warn).never + subject end end describe 'event' do context 'may?' do - class Base - def may_save?; end - def save!; end - def save; end + let(:base_klass) do + Class.new do + def may_save?; end + def save!; end + def save; end + end end - let(:klass) { Base } - it do - expect { Base.send :include, Clumsy }. - to output(/Base: overriding method 'may_save\?'!/).to_stderr - expect { Base.send :include, Clumsy }. - to output(/Base: overriding method 'save!'!/).to_stderr - expect { Base.send :include, Clumsy }. - to output(/Base: overriding method 'save'!/).to_stderr + + subject { base_klass.send :include, Clumsy } + + it 'should log to warn' do + expect_any_instance_of(Logger).to receive(:warn).exactly(3).times do |logger, message| + expect( + [ + ": overriding method 'may_save?'!", + ": overriding method 'save!'!", + ": overriding method 'save'!" + ] + ).to include(message) + end + subject end end end - end diff --git a/spec/unit/persistence/active_record_persistence_multiple_spec.rb b/spec/unit/persistence/active_record_persistence_multiple_spec.rb index 68a52e4..b4a1cba 100644 --- a/spec/unit/persistence/active_record_persistence_multiple_spec.rb +++ b/spec/unit/persistence/active_record_persistence_multiple_spec.rb @@ -384,6 +384,30 @@ describe 'transitions with persistence' do expect(validator).to be_valid expect(validator).to be_sleeping + validator.name = nil + expect(validator).not_to be_valid + expect { validator.run! }.to raise_error(ActiveRecord::RecordInvalid) + expect(validator).to be_sleeping + + validator.reload + expect(validator).not_to be_running + expect(validator).to be_sleeping + + validator.name = 'another name' + expect(validator).to be_valid + expect(validator.run!).to be_truthy + expect(validator).to be_running + + validator.reload + expect(validator).to be_running + expect(validator).not_to be_sleeping + end + + it 'should not store states for invalid models silently if configured' do + validator = MultipleSilentPersistor.create(:name => 'name') + expect(validator).to be_valid + expect(validator).to be_sleeping + validator.name = nil expect(validator).not_to be_valid expect(validator.run!).to be_falsey diff --git a/spec/unit/persistence/active_record_persistence_spec.rb b/spec/unit/persistence/active_record_persistence_spec.rb index 12c9d27..bedb621 100644 --- a/spec/unit/persistence/active_record_persistence_spec.rb +++ b/spec/unit/persistence/active_record_persistence_spec.rb @@ -385,6 +385,30 @@ describe 'transitions with persistence' do expect(validator).to be_valid expect(validator).to be_sleeping + validator.name = nil + expect(validator).not_to be_valid + expect { validator.run! }.to raise_error(ActiveRecord::RecordInvalid) + expect(validator).to be_sleeping + + validator.reload + expect(validator).not_to be_running + expect(validator).to be_sleeping + + validator.name = 'another name' + expect(validator).to be_valid + expect(validator.run!).to be_truthy + expect(validator).to be_running + + validator.reload + expect(validator).to be_running + expect(validator).not_to be_sleeping + end + + it 'should not store states for invalid models silently if configured' do + validator = SilentPersistor.create(:name => 'name') + expect(validator).to be_valid + expect(validator).to be_sleeping + validator.name = nil expect(validator).not_to be_valid expect(validator.run!).to be_falsey