Skip to content
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

Merged
merged 1 commit into from
Jan 3, 2015

Conversation

tanraya
Copy link
Contributor

@tanraya tanraya commented Dec 21, 2013

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 %>

@@ -1,3 +1,7 @@
* Use local variables in _form.html.erb generated by scaffold generator.

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.

@carlosantoniodasilva
Copy link
Member

👍

@dmathieu
Copy link
Contributor

@tanraya can you squash your commits please? Also, they don't apply cleanly into master. You'll need to rebase.

@tanraya
Copy link
Contributor Author

tanraya commented Dec 22, 2013

@dmathieu I will do.

@tanraya
Copy link
Contributor Author

tanraya commented Dec 28, 2013

@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)
Copy link
Member

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.

Choose a reason for hiding this comment

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

👍 for @senny's comment.

@jamo
Copy link
Contributor

jamo commented Feb 11, 2014

I added the suggested test case, see jamo@a0b548d

(all tests passes)

@arunagw
Copy link
Member

arunagw commented Jul 4, 2014

@tanraya did you get some time to update this PR?

thanks

@tanraya
Copy link
Contributor Author

tanraya commented Jul 25, 2014

@arunagw Yes. Updated now.

@arunagw
Copy link
Member

arunagw commented Jul 25, 2014

@tanraya seems test broke. Can you see those as well ?


<ul>
<%% @<%= singular_table_name %>.errors.full_messages.each do |message| %>
<li><%%= message %></li>
<%% end %>

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 %>
@tanraya
Copy link
Contributor Author

tanraya commented Jul 26, 2014

@carlosantoniodasilva @arunagw Now its done. All tests passed.

@rafaelfranca rafaelfranca modified the milestones: 4.2.0, 5.0.0 Aug 18, 2014
carlosantoniodasilva added a commit that referenced this pull request Jan 3, 2015
Use local variables in _form.html.erb generated by scaffold.

Conflicts:
	railties/CHANGELOG.md
carlosantoniodasilva added a commit that referenced this pull request Jan 3, 2015
@carlosantoniodasilva carlosantoniodasilva merged commit 6bd8126 into rails:master Jan 3, 2015
@rafaelfranca rafaelfranca modified the milestones: 5.0.0 [temp], 5.0.0 Dec 30, 2015
christiannelson added a commit to carbonfive/raygun-rails that referenced this pull request Jul 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants