Hobo Hobo → Hobo 1.1

Comments and changes to this ticket

• 

  Bryan Larsen April 1st, 2010 @ 03:35 PM

  Matt, Tom, do you have an opinion? I think he's right. Or is this too much of an API change for post-1.0?

• You flagged this item as spam.
  

  Betelgeuse April 1st, 2010 @ 03:42 PM

  There's the option of maintaining backwards compatibility by adding a configuration option to an initializer that gets added by default to new projects.

• 

  Matt Jones April 1st, 2010 @ 03:58 PM

  There's something weird going on here - some debugging may help. Here's the code for the transition's run! method:

        def run!(record, user, attributes)
          current_state = record.lifecycle.state_name
          unless start_states.include?(current_state)
            raise Hobo::Lifecycles::LifecycleError, "Transition #{record.class}##{name} cannot be run from the '#{current_state}' state"
          end
          record.lifecycle.active_step = self
          record.with_acting_user(user) do
            prepare!(record, attributes)
            if can_run?(record)
              if change_state(record)
                fire_event(record, on_transition)
              end
            else
              raise Hobo::PermissionDeniedError
            end
          end
        end
  

  The code appears to be trying to do this already, but not succeeding...

• You flagged this item as spam.
  

  Betelgeuse April 1st, 2010 @ 04:02 PM

  Actually from my debugging sessions I vaguely recall that run! wouldn't be on the call path for this case at all. At least I think the entry paths weren't really unified.

• 

  Tom Locke April 1st, 2010 @ 04:08 PM

  An exclamation mark method should throw an exception in my opinion like the rails create! method does.

  The (much longer standing) Ruby convention is that a ! method implies a side-effect (changing state in this case). e.g. array.map vs. array.map!. ActiveRecord is breaking the convention really. Though TBH it's not a strongly observed convention.

  However, I do agree that attempting the transition when in the wrong state should be a PermissionDenied error. An app that does this is definitely not following the rules, so I don't see it as a problem making this change. Maybe for 1.1 rather than for a maintenance release though?

• 

  Matt Jones April 1st, 2010 @ 04:32 PM

  Oh wait, you're right - the call to find_transition on line 128 of lifecycle.rb will return nil and not run the transition at all.

• 

  Tom Locke April 1st, 2010 @ 05:52 PM

  Should maybe be more like

  def transition(name, user, attributes)
    transition = find_transition(name, user) or raise LifecycleError, "No transition #{name} available"
    transition.run!(record, user, attributes)
  end
  

  ?

• 

  Bryan Larsen April 12th, 2010 @ 09:23 PM

  I presume the same change should be made on self.create?

• 

  Bryan Larsen April 12th, 2010 @ 09:26 PM

  • State changed from “new” to “open”
  • Tag changed from “lifecycles” to “defect, lifecycles”
  • Milestone set to “Hobo 1.1”

  self.create already throws an exception if the creator is not available. The change will make it friendlier, though.

• 

  Bryan Larsen April 13th, 2010 @ 05:50 PM

  • State changed from “open” to “resolved”

  (from [68ebb535d37843a03d5d53120ad5b9ae1f389510]) [#674 state:resolved] raise exception when trying to run an invalid lifecycle transition http://github.com/tablatom/hobo/commit/68ebb535d37843a03d5d53120ad5...