Hobo Hobo → Hobo 1.0 - Final

Comments and changes to this ticket

• 

  Tom Locke October 29th, 2009 @ 08:29 PM

  • State changed from “new” to “open”
• 

  Bryan Larsen November 24th, 2009 @ 08:03 PM

  It looks like the changes to user.rb in http://github.com/tablatom/hobo/commit/fe29c62d67b92443198a643a6608... has been reverted, that distracted me for a bit.

  Just to be clear, it's not anything functional that's bothering you, it's just the flash message in hobo_do_signup not corresponding to the redirect to account_disabled in hobo_login?

  I actually don't think that conflict is very serious -- you can't login to a new account until it has a password, so a new user will never be redirected to account_disabled.

  And hobo_do_signup should only fire on new users, so that makes sense too.

  Imagine a three-state user model: unconfirmed, active, and disabled. The existing code should work with that. Perhaps I should change agility to reflect all three states?

  One small thing: maybe account_active? should be moved into the generated user code so that it's clear that users can override it. (actually copying will avoid breaking code)

  ran across this interesting commit: http://github.com/tablatom/hobo/commit/b27c731aae172c048c54f160e7fc... I'm not sure what that's supposed to solve!

• 

  Tom Locke November 26th, 2009 @ 03:46 PM

  OK so what we're saying here is that #account_active? can return false in both cases (not active due to needing to click the email link, and not active due to the admin having disabled your account)

  That's OK but I still object a bit because in fact Hobo doesn't directly support email activation at all. It's easy to implement with lifecycles, and that's quite deliberate of course, but I don't think the flash message "You must activate your account before you can log in. Please check your email" should be in core hobo.

  I propose removing the account_active? stuff from hobo_do_signup altogether. It's easy to add back in by passing a block.

• 

  Matt Jones November 26th, 2009 @ 04:13 PM

  I think the account_active? check in the second half of hobo_do_signup is important - the message isn't necessarily correct, but the check before setting current_user seems sensible. There's lots of reasons that a user might not be able to immediately sign in; maybe email activation, maybe an admin needs to approve their account, etc. account_active? essentially encodes that status.

  Although the existing test is somewhat... redundant. An if block with a statement with a matching if modifier??

• 

  Bryan Larsen November 26th, 2009 @ 05:39 PM

  Tom, I agree with you. I guess the question would be -- what is an appropriate change at this stage? Many apps will be depending on current behaviour.

  I would argue that the flash messages should be in the user controller template rather than in user_controller.rb.

  Should we make that change and call it out in the CHANGES & blog posting?

• 

  Tom Locke November 26th, 2009 @ 06:02 PM

  I agree with Matt that it's nice to let that hook be used to disable login-on-signup. I think we just change the flash message to a generic "You account is not yet active". Of course that's fairly useless but it's not hard to change.

  We can just announce it as a breaking change.

• 

  Bryan Larsen November 26th, 2009 @ 06:57 PM

  I removed the comment entirely. I think most applications would want to add a little more text than "Your account is not yet active". I'll fix up agility to point this out.

• 

  Bryan Larsen November 30th, 2009 @ 02:47 PM

  • State changed from “open” to “resolved”
  • Tag cleared.

  I closed this with 21201689b0f7669005c6159ff9ecae76cd348243, but the github<->lighthouse integration didn't pick it up.