From ea15f8e6dfcdefcc6439e712f6e0fb21874b934c Mon Sep 17 00:00:00 2001 From: Jonathan Hefner Date: Wed, 15 Dec 2021 12:33:14 -0600 Subject: [PATCH] Fix scaffold index.html.erb variable name Due to a conflict between #43846 and #43611, the variable name generated inside the `each` block did not match the block parameter name for namespaced models. For example, if the block parameter name was `admin_role`, the variable name inside the block would be just `role`. This commit changes `index.html.erb` to use the correct variable name, and adds much more test coverage. This commit also changes the target for the "Destroy" button rendered in `show.html.erb` from a route helper call to a bare model reference, e.g. from `admin_role_path(@admin_role)` to just `@admin_role`. This was missed in #43611. --- .../erb/scaffold/templates/index.html.erb.tt | 2 +- .../erb/scaffold/templates/show.html.erb.tt | 2 +- railties/lib/rails/generators/named_base.rb | 4 +- .../scaffold_controller_generator_test.rb | 40 ++++++++++++------- .../generators/scaffold_generator_test.rb | 28 +++++++++++-- 5 files changed, 54 insertions(+), 22 deletions(-) diff --git a/railties/lib/rails/generators/erb/scaffold/templates/index.html.erb.tt b/railties/lib/rails/generators/erb/scaffold/templates/index.html.erb.tt index 938d742c82..25ecac400f 100644 --- a/railties/lib/rails/generators/erb/scaffold/templates/index.html.erb.tt +++ b/railties/lib/rails/generators/erb/scaffold/templates/index.html.erb.tt @@ -6,7 +6,7 @@ <%% @<%= plural_table_name %>.each do |<%= singular_table_name %>| %> <%%= render <%= singular_table_name %> %>

- <%%= link_to "Show this <%= human_name.downcase %>", <%= model_resource_name(singular_name) %> %> + <%%= link_to "Show this <%= human_name.downcase %>", <%= model_resource_name(singular_table_name) %> %>

<%% end %> diff --git a/railties/lib/rails/generators/erb/scaffold/templates/show.html.erb.tt b/railties/lib/rails/generators/erb/scaffold/templates/show.html.erb.tt index e9790bcac9..3187f5e004 100644 --- a/railties/lib/rails/generators/erb/scaffold/templates/show.html.erb.tt +++ b/railties/lib/rails/generators/erb/scaffold/templates/show.html.erb.tt @@ -6,5 +6,5 @@ <%%= link_to "Edit this <%= human_name.downcase %>", <%= edit_helper(type: :path) %> %> | <%%= link_to "Back to <%= human_name.pluralize.downcase %>", <%= index_helper(type: :path) %> %> - <%%= button_to "Destroy this <%= human_name.downcase %>", <%= show_helper(type: :path) %>, method: :delete %> + <%%= button_to "Destroy this <%= human_name.downcase %>", <%= model_resource_name(prefix: "@") %>, method: :delete %> diff --git a/railties/lib/rails/generators/named_base.rb b/railties/lib/rails/generators/named_base.rb index 15ab2fdabc..3039ed8526 100644 --- a/railties/lib/rails/generators/named_base.rb +++ b/railties/lib/rails/generators/named_base.rb @@ -147,8 +147,8 @@ module Rails model_resource_name(prefix: "@") end - def model_resource_name(model = singular_table_name, prefix: "") # :doc: - resource_name = "#{prefix}#{model}" + def model_resource_name(base_name = singular_table_name, prefix: "") # :doc: + resource_name = "#{prefix}#{base_name}" if options[:model_name] "[#{controller_class_path.map { |name| ":" + name }.join(", ")}, #{resource_name}]" else diff --git a/railties/test/generators/scaffold_controller_generator_test.rb b/railties/test/generators/scaffold_controller_generator_test.rb index d0a6e0c8dc..5dfac34563 100644 --- a/railties/test/generators/scaffold_controller_generator_test.rb +++ b/railties/test/generators/scaffold_controller_generator_test.rb @@ -221,27 +221,37 @@ class ScaffoldControllerGeneratorTest < Rails::Generators::TestCase end assert_file "app/views/admin/users/index.html.erb" do |content| - assert_match(%{"New user", new_admin_user_path}, content) - assert_match(%{"Show this user", [:admin, user]}, content) - end - - assert_file "app/views/admin/users/new.html.erb" do |content| - assert_match(%{"Back to users", admin_users_path}, content) - end - - assert_file "app/views/admin/users/edit.html.erb" do |content| - assert_match(%{"Show this user", [:admin, @user]}, content) - assert_match(%{"Back to users", admin_users_path}, content) + assert_match %{@users.each do |user|}, content + assert_match %{render user}, content + assert_match %{"Show this user", [:admin, user]}, content + assert_match %{"New user", new_admin_user_path}, content end assert_file "app/views/admin/users/show.html.erb" do |content| - assert_match(%{"Edit this user", edit_admin_user_path(@user)}, content) - assert_match(%{"Back to users", admin_users_path}, content) - assert_match(%{"Destroy this user", admin_user_path(@user)}, content) + assert_match %{render @user}, content + assert_match %{"Edit this user", edit_admin_user_path(@user)}, content + assert_match %{"Back to users", admin_users_path}, content + assert_match %{"Destroy this user", [:admin, @user]}, content + end + + assert_file "app/views/admin/users/_user.html.erb" do |content| + assert_match "user", content + assert_no_match "admin_user", content + end + + assert_file "app/views/admin/users/new.html.erb" do |content| + assert_match %{render "form", user: @user}, content + assert_match %{"Back to users", admin_users_path}, content + end + + assert_file "app/views/admin/users/edit.html.erb" do |content| + assert_match %{render "form", user: @user}, content + assert_match %{"Show this user", [:admin, @user]}, content + assert_match %{"Back to users", admin_users_path}, content end assert_file "app/views/admin/users/_form.html.erb" do |content| - assert_match("model: [:admin, user]", content) + assert_match %{model: [:admin, user]}, content end assert_file "test/controllers/admin/users_controller_test.rb" do |content| diff --git a/railties/test/generators/scaffold_generator_test.rb b/railties/test/generators/scaffold_generator_test.rb index a63bd55d66..24c2ca2bd7 100644 --- a/railties/test/generators/scaffold_generator_test.rb +++ b/railties/test/generators/scaffold_generator_test.rb @@ -270,7 +270,17 @@ class ScaffoldGeneratorTest < Rails::Generators::TestCase # Views assert_file "app/views/admin/roles/index.html.erb" do |content| - assert_match(%("New role", new_admin_role_path), content) + assert_match %{@admin_roles.each do |admin_role|}, content + assert_match %{render admin_role}, content + assert_match %{"Show this role", admin_role}, content + assert_match %{"New role", new_admin_role_path}, content + end + + assert_file "app/views/admin/roles/show.html.erb" do |content| + assert_match %{render @admin_role}, content + assert_match %{"Edit this role", edit_admin_role_path(@admin_role)}, content + assert_match %{"Back to roles", admin_roles_path}, content + assert_match %{"Destroy this role", @admin_role}, content end assert_file "app/views/admin/roles/_role.html.erb" do |content| @@ -278,9 +288,21 @@ class ScaffoldGeneratorTest < Rails::Generators::TestCase assert_no_match "admin_role", content end - %w(edit new show _form).each do |view| - assert_file "app/views/admin/roles/#{view}.html.erb" + assert_file "app/views/admin/roles/new.html.erb" do |content| + assert_match %{render "form", admin_role: @admin_role}, content + assert_match %{"Back to roles", admin_roles_path}, content end + + assert_file "app/views/admin/roles/edit.html.erb" do |content| + assert_match %{render "form", admin_role: @admin_role}, content + assert_match %{"Show this role", @admin_role}, content + assert_match %{"Back to roles", admin_roles_path}, content + end + + assert_file "app/views/admin/roles/_form.html.erb" do |content| + assert_match %{model: admin_role}, content + end + assert_no_file "app/views/layouts/admin/roles.html.erb" # Helpers