This project is archived and is in readonly mode.

#111 ✓resolved
Markús

Default path names ignored

Reported by Markús | May 4th, 2008 @ 10:10 PM | in 2.1.1

In changes 063c393bf0a2eb762770c97f925b7c2867361ad4 and e6a3ce3392812f707b78d64ffb04ee52f4517d20 a condition in map_member_action make Base.resources_path_names (default path names) being ignored:

in resources.rb (line 527):

action_path = action
if resource.options[:path_names]
   action_path = resource.options[:path_names][action]
   action_path ||= Base.resources_path_names[action] || action
end

That "if resource.options[:path_names]" should not be there, it should be like this...

action_path = resource.options[:path_names][action] if resource.options[:path_names]
action_path ||= Base.resources_path_names[action] || action

...which whould make default path names work properly.

Comments and changes to this ticket

  • Michael Koziarski

    Michael Koziarski May 4th, 2008 @ 10:58 PM

    • Milestone set to 2.1.1

    Could you include a test case too please? otherwise we run the risk of breaking it again.

    Also if you follow the guide here it'll make it easier to get your the credits in the history.

  • Markús

    Markús May 4th, 2008 @ 11:29 PM

    Well Michael, I think this little change do not need a indiviual test case in the rails tests. This change is a refactoring, not an addition of features. As you see in the example in this ticket, the "if" statement is redundant and make something like this:

    # considering as resources_path_names = { :new => 'nuevo', :edit => 'editar' } and
    # the resource definition is map.resources :things, :as => 'cosas'
    
    # if action here is, for example, 'edit', it is asigned to action_path
    action_path = action
    # options[:path_names] is not defined in the resource declaration, threrefore the action_path remains as 'edit'
    if resource.options[:path_names]
       action_path = resource.options[:path_names][action]
       # Here resides the bug
       # The apperance of "action" here is redundant, since it was asigned to action_path before
       # Base.resource_path_names[action] is not evaluated unless the resource declaration has the :path_names parameter defined directly, 
       action_path ||= Base.resources_path_names[action] || action
    end
    
    # in rake:routes the route appears as 'cosas/thing_id/edit'
    

    Well, now, with my refactorization:

    # Only evalue resource.options[:path_names] if exists
    action_path = resource.options[:path_names][action] if resource.options[:path_names]
    # Perform default asignment
    # If previous line was evaluated, this line do nothing
    # If previous line was not evaluated and exists a Base.resources_path_names for this action, the value is asigned
    # If none of the previous examples were evaluated, then the action_path is the action itslef (as ocurrs with the first line the original code)
    action_path ||= Base.resources_path_names[action] || action
    
    # considering the same definitions in the previous example, here the rake:routes shows 'cosa/thing_id/editar' as spected
    
  • Markús

    Markús May 5th, 2008 @ 09:05 AM

    I've created a new patch with a new test reflecting the bug I've found and ensuring the correct use of ActionController::Base.resources_path_names. I've followed the guide for contributing to rails.

    If you finally apply this patch and you want to reference me, my GitHub profile is yizzreel

    Cheers

  • Repository

    Repository May 6th, 2008 @ 11:01 AM

    • State changed from “new” to “resolved”

    (from [2c39836dc3c06813fce031d1bb390149b53ebd1c]) Refactored and fixed Resources.map_member_actions to make use of custom ActionController::Base.resources_path_names when the option :path_names is not directly specified. Added a specific test for this functionality and fixed assert_restful_routes_for test helper to make use of ActionController::Base.resources_path_names instead of just "new" or "edit".

    Signed-off-by: Michael Koziarski

    [#111 state:resolved]

    http://github.com/rails/rails/co...

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Attachments

Pages