Skip to content

Commit

Permalink
accept invitation should set the flash only if this.valid? [#930 stat…
Browse files Browse the repository at this point in the history 
…e:resolved]
  • Loading branch information
@ddnexus
ddnexus committed Mar 27, 2011
1 parent 578a8ab commit f9c2770
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions hobo/lib/generators/hobo/user_controller/templates/controller.rb.erb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ class <%= class_name %>Controller < ApplicationController

def do_accept_invitation
do_transition_action :accept_invitation do
self.current_user = this
flash[:notice] = t("hobo.messages.you_signed_up", :default=>"You have signed up")
flash[:notice] = t("hobo.messages.you_signed_up", :default=>"You have signed up") if this.valid?
end
end
<% end -%>
Expand Down

4 comments on commit f9c2770

@bryanlarsen
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you mean to delete the self.current_user = this as well? This is a significant behavior change. Maybe the right one, but it will change the way a lot of apps work...

@ddnexus
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bryan,
it should not change any behaviour of any old app, since it is a change in a generator template, so it affects only the UserController of the new created app: there is no change in the hobo framework itself.
As for the fact that I deleted it: if you try to stop at line 19, self.current_user is already set: indeed it is set by the key in the url that triggers the do_accept_invitation, so IMHO it is just redundant. Am I missing something?

@ddnexus
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-thinkig about the fact that it is already set: it should not be set. I have to investigate: may be it has something to do with https://hobo.lighthouseapp.com/projects/8324/tickets/718-lifecycle-transitions-with-validation-errors-should-not-update-the-state-field, or the bug that Tom reported a few days ago.

@ddnexus
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No bugs, just the logged in user in development mode made me think something stupid! :-)
The current_user is correctly set to guest, and this is the user record, so everything is ok, I have just to restore the statement :-)
Thank you.

Please sign in to comment.