-
-
Save godfat/20d38d81cc2b815913b7 to your computer and use it in GitHub Desktop.
In reply to: https://twitter.com/plataformatec/status/525670283899072513 | |
Hi, | |
This would probably be a long story, and I am not going to go | |
through all the details here. You're doing well in what | |
could be done, but in general, I oppose to build a user system | |
on top of some generic framework. The reason is that a user | |
system could be very specific to business logic, and it | |
could be very hard to customize devise in a way that works | |
well for that. | |
Honestly this could simply because I am not familiar with how | |
devise works, but it's really frustrating to track devise code. | |
For example, now I am getting this message when trying to do: | |
`sign_in_and_redirect(user)` | |
You have to confirm your account before continuing. | |
[updated: This is now fixed. See comments below. It's totally my own fault] | |
And I have no idea where this was triggered. The user passed | |
definitely has `confirmed_at` set, and the user could | |
successfully login with the form when tried again. It's simply | |
that calling `sign_in_and_redirect(user)` didn't work. | |
We're also stuck at 3.0.4 and cannot upgrade right now due to | |
the flow change from later devise, and I don't yet find a way to | |
make it work the same way for now. I believe this could be done, | |
but it's just costing me too much time and effort than | |
trying to do on my own. | |
In short, I don't think such user system would work if we're | |
building a business which would require a bunch of customization, | |
unless I am so familiar with devise to begin with. (for example, | |
if I developed devise, then of course I could find a way to do it) | |
and I think surely devise could have better implemented somehow. | |
I guess this is simply too ambitious and could hardly do right. | |
It's improving over time, definitely, but we're already stuck there... | |
To show a quick implementation improvement: | |
https://github.com/hassox/warden/blob/v1.2.3/lib/warden/proxy.rb#L212 | |
I believe it's not a good idea to name a local variable the same as | |
the defining method. Without looking though line-by-line, it could look | |
like doing some crazy recursion. Which is not. | |
I saw things like that tracking through warden and devise. | |
Really a bad experience for me. | |
My two cents. | |
Thanks for listening to my complaining. | |
Also a quick suggestion for: | |
http://blog.plataformatec.com.br/2013/08/devise-3-1-now-with-more-secure-defaults/ | |
Regarding "Store digested tokens in the database": | |
Since we're already using bcrypt, I think we could just use bcrypt for | |
this purpose. Just treat confirmation tokens as password, and of course | |
remove the tokens after it's used. |
I detected that it's throwing :warden
with {:scope=>:user, :message=>:unconfirmed}
But really, where is this sign_in_and_redirect
defined? Why I can't find it?
Isn't that in devise helper?
A related question and no one answered:
http://stackoverflow.com/questions/23756102/devise-confirmable-confirmation-error
Stupid me. Just realized that I was reading devise 3.3.0 instead of 3.0.4.
Finally tracked it down after looking at the right code.
It's because there's no update_columns
in rails 3,
and so I was using self.class.where(:id => id).update_all(args)
,
and then the instance is not reflecting the value from database.
Silly me. It's totally my fault. After putting a quick reload
now it works.
Should probably just change the instance without reloading though.
Replaced reload
to: args.each{ |k, v| send("#{k}=", v) }
I put some
puts
insign_in_and_redirect
, and it's not called.Huh? Then which method is
sign_in_and_redirect
really calling???