Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

Default path names ignored

#111

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.

Reported by Markús · May 4th, 2008 @ 10:10 PM

State: resolved
Milestone: 2.1.1
Assigned to: Michael Koziarski Michael Koziarski
Importance: none

Activity

  1. Michael Koziarski
    Michael Koziarski
    • 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.

    May 4th, 2008 @ 10:58 PM

  2. Markús
    Markús

    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
    

    May 4th, 2008 @ 11:29 PM

  3. Markús
    Markús

    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

    May 5th, 2008 @ 09:05 AM

  4. Repository
    Repository
    • 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...

    May 6th, 2008 @ 11:01 AM