-
-
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
putsinsign_in_and_redirect, and it's not called.Huh? Then which method is
sign_in_and_redirectreally calling???