Skip to content

Commit

Permalink
Merge PR #43611
Browse files Browse the repository at this point in the history
Squashed commit of the following:

    Tweaks

    Extend test adding more assertions

    Update scaffold generator test and add application generator test for controller generator with namespace

    Adapt code to feedback suggestions

    Apply suggestions from code review

    Run generated tests by scaffold controller generator

    Fix generated tests too

    Keep generated url helpers consistent

    Fix scaffold controller generator with namespace

    For example:

    ```
    bundle exec rails g scaffold_controller Admin::Post title:string content:text --model-name=Post
    ```

    It fixes the code generated by railties, other changes are required on
    jbuilder gem.

Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
Co-authored-by: José Galisteo <ceritium@gmail.com>
  • Loading branch information
3 people committed Dec 15, 2021
1 parent 0776c8b commit 6cd75a4
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 25 deletions.
Expand Up @@ -5,6 +5,6 @@
<br>

<div>
<%%= link_to "Show this <%= human_name.downcase %>", @<%= singular_table_name %> %> |
<%%= link_to "Back to <%= human_name.pluralize.downcase %>", <%= index_helper %>_path %>
<%%= link_to "Show this <%= human_name.downcase %>", <%= model_resource_name(prefix: "@") %> %> |
<%%= link_to "Back to <%= human_name.pluralize.downcase %>", <%= index_helper(type: :path) %> %>
</div>
Expand Up @@ -6,9 +6,9 @@
<%% @<%= plural_table_name %>.each do |<%= singular_table_name %>| %>
<%%= render <%= singular_table_name %> %>
<p>
<%%= link_to "Show this <%= human_name.downcase %>", <%= singular_name %> %>
<%%= link_to "Show this <%= human_name.downcase %>", <%= model_resource_name(singular_name) %> %>
</p>
<%% end %>
</div>

<%%= link_to "New <%= human_name.downcase %>", new_<%= singular_route_name %>_path %>
<%%= link_to "New <%= human_name.downcase %>", <%= new_helper(type: :path) %> %>
Expand Up @@ -5,5 +5,5 @@
<br>

<div>
<%%= link_to "Back to <%= human_name.pluralize.downcase %>", <%= index_helper %>_path %>
<%%= link_to "Back to <%= human_name.pluralize.downcase %>", <%= index_helper(type: :path) %> %>
</div>
Expand Up @@ -3,8 +3,8 @@
<%%= render @<%= singular_table_name %> %>

<div>
<%%= link_to "Edit this <%= human_name.downcase %>", edit_<%= singular_table_name %>_path(@<%= singular_table_name %>) %> |
<%%= link_to "Back to <%= human_name.pluralize.downcase %>", <%= index_helper %>_path %>
<%%= 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 %>", <%= singular_table_name %>_path(@<%= singular_table_name %>), method: :delete %>
<%%= button_to "Destroy this <%= human_name.downcase %>", <%= show_helper(type: :path) %>, method: :delete %>
</div>
20 changes: 10 additions & 10 deletions railties/lib/rails/generators/named_base.rb
Expand Up @@ -94,20 +94,20 @@ def uncountable? # :doc:
singular_name == plural_name
end

def index_helper # :doc:
uncountable? ? "#{plural_route_name}_index" : plural_route_name
def index_helper(type: nil) # :doc:
[plural_route_name, ("index" if uncountable?), type].compact.join("_")
end

def show_helper # :doc:
"#{singular_route_name}_url(@#{singular_table_name})"
def show_helper(arg = "@#{singular_table_name}", type: :url) # :doc:
"#{singular_route_name}_#{type}(#{arg})"
end

def edit_helper # :doc:
"edit_#{show_helper}"
def edit_helper(...) # :doc:
"edit_#{show_helper(...)}"
end

def new_helper # :doc:
"new_#{singular_route_name}_url"
def new_helper(type: :url) # :doc:
"new_#{singular_route_name}_#{type}"
end

def singular_table_name # :doc:
Expand Down Expand Up @@ -147,8 +147,8 @@ def redirect_resource_name # :doc:
model_resource_name(prefix: "@")
end

def model_resource_name(prefix: "") # :doc:
resource_name = "#{prefix}#{singular_table_name}"
def model_resource_name(model = singular_table_name, prefix: "") # :doc:
resource_name = "#{prefix}#{model}"
if options[:model_name]
"[#{controller_class_path.map { |name| ":" + name }.join(", ")}, #{resource_name}]"
else
Expand Down
Expand Up @@ -11,7 +11,7 @@ class <%= controller_class_name %>ControllerTest < ActionDispatch::IntegrationTe
end

test "should get index" do
get <%= index_helper %>_url
get <%= index_helper(type: :url) %>
assert_response :success
end

Expand All @@ -22,10 +22,10 @@ class <%= controller_class_name %>ControllerTest < ActionDispatch::IntegrationTe

test "should create <%= singular_table_name %>" do
assert_difference("<%= class_name %>.count") do
post <%= index_helper %>_url, params: { <%= "#{singular_table_name}: { #{attributes_string} }" %> }
post <%= index_helper(type: :url) %>, params: { <%= "#{singular_table_name}: { #{attributes_string} }" %> }
end

assert_redirected_to <%= singular_table_name %>_url(<%= class_name %>.last)
assert_redirected_to <%= show_helper("#{class_name}.last") %>
end

test "should show <%= singular_table_name %>" do
Expand All @@ -40,15 +40,15 @@ class <%= controller_class_name %>ControllerTest < ActionDispatch::IntegrationTe

test "should update <%= singular_table_name %>" do
patch <%= show_helper %>, params: { <%= "#{singular_table_name}: { #{attributes_string} }" %> }
assert_redirected_to <%= singular_table_name %>_url(<%= "@#{singular_table_name}" %>)
assert_redirected_to <%= show_helper %>
end

test "should destroy <%= singular_table_name %>" do
assert_difference("<%= class_name %>.count", -1) do
delete <%= show_helper %>
end

assert_redirected_to <%= index_helper %>_url
assert_redirected_to <%= index_helper(type: :url) %>
end
end
<% end -%>
29 changes: 27 additions & 2 deletions railties/test/generators/scaffold_controller_generator_test.rb
Expand Up @@ -221,16 +221,41 @@ def test_model_name_option
end

assert_file "app/views/admin/users/index.html.erb" do |content|
assert_match("\"New user\", new_admin_user_path", content)
assert_match(%{"New user", new_admin_user_path}, content)
end

assert_file "app/views/admin/users/new.html.erb" do |content|
assert_match("\"Back to users\", admin_users_path", 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)
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)
end

assert_file "app/views/admin/users/_form.html.erb" do |content|
assert_match("model: [:admin, user]", content)
end

assert_file "app/views/admin/users/_user.html.erb" do |content|
assert_match(%{"Show this user", [:admin, user]}, content)
end

assert_file "test/controllers/admin/users_controller_test.rb" do |content|
assert_match " admin_users_url", content
assert_match " new_admin_user_url", content
assert_match " edit_admin_user_url", content
assert_match " admin_user_url(@user)", content
assert_no_match %r/\b(new_|edit_)?users?_(path|url)/, content
end

assert_file "test/system/users_test.rb"
end

def test_controller_tests_pass_by_default_inside_mountable_engine
Expand Down

0 comments on commit 6cd75a4

Please sign in to comment.