-
Notifications
You must be signed in to change notification settings - Fork 21.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use local variables in _form.html.erb generated by scaffold. #13434
Conversation
@@ -1,3 +1,7 @@ | |||
* Use local variables in _form.html.erb generated by scaffold generator. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
`_form.html.erb` partial generated by scaffold.
👍 |
@tanraya can you squash your commits please? Also, they don't apply cleanly into master. You'll need to rebase. |
@dmathieu I will do. |
@dmathieu done |
@@ -68,6 +68,17 @@ def test_scaffold_on_invoke | |||
end | |||
assert_no_file "app/views/layouts/product_lines.html.erb" | |||
|
|||
# Views local variables | |||
assert_file "app/views/product_lines/_form.html.erb" do |test| | |||
assert_no_match(/@product_line/, test) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should verify what should be there, not what shouldn't be there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 for @senny's comment.
I added the suggested test case, see jamo@a0b548d (all tests passes) |
@tanraya did you get some time to update this PR? thanks |
@arunagw Yes. Updated now. |
@tanraya seems test broke. Can you see those as well ? |
|
||
<ul> | ||
<%% @<%= singular_table_name %>.errors.full_messages.each do |message| %> | ||
<li><%%= message %></li> | ||
<%% end %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems this should not be removed.
It seems a good idea to use local variables in generated partials instead of using instance variables. Before <%= render 'form' %> After <%= render 'form', product: @Product %>
@carlosantoniodasilva @arunagw Now its done. All tests passed. |
Use local variables in _form.html.erb generated by scaffold. Conflicts: railties/CHANGELOG.md
See rails/rails#13434 for reference.
It seems a good idea to use local variables in generated partials instead of using instance variables.
Before
After