Add support for generate scaffold password:digest #9699
Conversation
|
I like it. I have a question, do we need the password digest on the index and show? |
| @@ -2,8 +2,12 @@ class <%= migration_class_name %> < ActiveRecord::Migration | |||
| def change | |||
| create_table :<%= table_name %> do |t| | |||
| <% attributes.each do |attribute| -%> | |||
| <% if attribute.name == 'password' and attribute.type == :digest -%> | |||
There was a problem hiding this comment.
I think we could move the logic for attr name and type to the GeneratedAttribute class, so that the logic is in one place and you can check like attribute.password_digest? or something like that.
|
My initial thinking was that it was easier for an application developer to remove password digest lines from a template than it is to add them, but if most will remove them, and if adding these lines is both obvious and easy, then I can buy the argument that these lines shouldn't be added in the first place. Removed. I've also refactored the password digest attribute check based on Carlos' suggestion. |
|
|
|
With the addition of a test case and a changelog entry, my recommendation is that this be pulled now. Updating jquery-rails, the edge guides, and my test suite can proceed after that is complete. |
|
squashed |
| <strong><%= attribute.human_name %>:</strong> | ||
| <%%= @<%= singular_table_name %>.<%= attribute.name %> %> | ||
| <% end -%> | ||
| </p> |
There was a problem hiding this comment.
This will generate an empty <p> tag for the digest attribute, can we move the check to avoid it?
|
Looks great |
* adds password_digest attribute to the migration * adds has_secure_password to the model * adds password and password_confirmation password_fields to _form.html * omits password entirely from index.html and show.html * adds password and password_confirmation to the controller * adds unencrypted password and password_confirmation to the controller test * adds encrypted password_digest to the fixture
Add support for generate scaffold password:digest * adds password_digest attribute to the migration * adds has_secure_password to the model * adds password and password_confirmation password_fields to _form.html * omits password from index.html and show.html * adds password and password_confirmation to the controller * adds unencrypted password and password_confirmation to the controller test * adds encrypted password_digest to the fixture
|
|
|
|
|
|
has_secure_password is a useful feature that presents "virtual" password and password_confirmation attributes on creation and update backed by a password_digest attribute in the database. Getting the migration, model, views, tests, and fixtures correct is needlessly complicated. This change enables application developers to specify password:digest on a generate scaffold command. Doing so:
adds password_digest to index.html and show.htmlThe following tests have been run and pass successfully:
Additionally, the unmodified Agile Web Development with Rails tests have been run (i.e., they haven't been updated to use this new option), and I've verified that absolutely no change to any of the generated artefacts results by this change.
Not done yet:
changelog, new tests, jbuilder, and updating the edge guides and the AWDwR tests.@steveklabnik, @jeremy, @carlosantoniodasilva