Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Absolute partial paths need to be preserved in namespace #6478

Closed
wants to merge 1 commit into from

9 participants

@bopm

I got feeling that solution for #1951 are incorrect by nature. We include path namespacing in partial so even if we define custom resolver or just modify append_view_path we can't avoid namespacing. I am not sure how to fix this, but for my needs (defining one partial template for front/backend of my application using to_partial_path) this simple fix works.

@josevalim
Owner

@bopm Thanks for the pull request.

I must admit the partial lookup rules today are ultimately fucked up. We are trying to be too smart, which means that, for anyone to actually understand how a partial is rendered, he needs to at least answer 3 or 4 questions:

1) Does the partial path has a slash?
2) Is namespaced prefix enabled? (The default is yes)
3) Is the current controller namespaced?
4) Is the current partial namespaced to the current controller?

Since Rails 4 is coming, I would like to simplify how partials are rendered. In my opinion, we have two simple approaches:

1) Make all partials by default absolute. Therefore, when you pass render :partial => "form", it will try to render form in app/views. We could force a relative lookup by prepending "./".

2) Make all partials by default relative. Therefore, when you pass render :partial => "posts/post", it will try to render it relative to the current controller. You would be able to get an absolute path by prepending a "/".

The first alternative seems to be more disruptive to me and therefore I like 2) more. Regardless, the nice thing about any of these approaches is that the object being rendered will be able to specify if it wants a relative or absolute lookup and it is no longer intrinsic to the name.

I can prepare a patch myself, but I would like some feedback before. /cc @jeremy

@rafaelfranca

I prefer the option 2).

@steveklabnik
Collaborator

:+1: for making this simpler, and #2 as a solution.

@steveklabnik
Collaborator

So, is this ever going to get accepted, @josevalim, or not? I think if I'm reading you right, the answer is no.

@drogus
Collaborator

Also, do we decide to implement one of the options that @josevalim mentioned? There is also a related ticket #1143, that mentions adding ./ as a way to use relative path.

While I like the idea of making things simpler, this would unfortunately break almost all of the apps and would make the update to 4.0 much harder. That's why I would vote to implement #1143 (I think it will be quite easy to add, I can implement if it's the way to go).

@spastorino
Owner

Hmm I like @josevalim option 2). But I see that we will be breaking apps.
Could be nice if we can list which use cases would break if we apply José's solution.

@rafaelfranca

@drogus if #1143 make easier to upgrade I vote to implement it.

@drogus
Collaborator

@rafaelfranca after thinking about that I would implement it, then deprecate the old behavior and release a plugin that changes the behavior for the new one, so people can upgrade before 4.1. That way, we will deprecate in 4.0 and give people easier path to upgrade (so they can prepare everything before 4.1 comes out). Sounds good?

UPDATE:

of course when I wrote deprecate, I meant, deprecate in 4.0.

@drogus
Collaborator

I will try to come up with a pull request and we will see how it looks like :)

@kristianmandrup

Amazing that this has never received the attention that it deserves. I created a partializer gem in order to facilitate the partials path hell, when going beyond one nested level of partials. I don't like any file to be much more than ~60 lines, then I discovered Cells which is a much nicer solution. The views/partials in Rails is still such a primitive solution and something like Cells (or apotomo) really ought to be built into Rails IMO. Most MVC frameworks suffer from a lack of proper view partitioning. I recall an even worse nightmare on the Java stack. Time to make Rails a little more mature, no?

@drogus drogus was assigned
@JonRowe

Option 2 gets my :+1:

There needs to be some way to force an absolute lookup for namespaced partials, to_partial_path is essentially being ignored in namespaces.

@steveklabnik
Collaborator

It seems that everyone is interested in option 2, and with the intention not to break people's apps, I'm giving this a close. An implementation of option 2 would make a great new PR though. :)

@ncri

Any news on this? Has anything been implemented? I can still not get relative paths working properly when partial nesting is deeper than one level.

@kristianmandrup

@ncri, you can try my "partializer" gem, which allows for a different approach to achieve this.

@ncri

Thanks, I'll try that. Would be nice though if one doesn't have to rely on a gem to simply have relative partial access from the render method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 24, 2012
  1. @bopm
This page is out of date. Refresh to see the latest.
View
2  actionpack/lib/action_view/renderer/partial_renderer.rb
@@ -431,7 +431,7 @@ def prefixed_partial_names
end
def merge_prefix_into_object_path(prefix, object_path)
- if prefix.include?(?/) && object_path.include?(?/)
+ if prefix.include?(?/) && object_path.include?(?/) && object_path[0] != '/'
prefixes = []
prefix_array = File.dirname(prefix).split('/')
object_path_array = object_path.split('/')[0..-3] # skip model dir & partial
View
12 actionpack/test/controller/render_test.rb
@@ -19,6 +19,10 @@ class QuestionsController < ActionController::Base
def new
render :partial => Quiz::Question.new("Namespaced Partial")
end
+
+ def absolute
+ render :partial => Quiz::Absolute.new
+ end
end
end
@@ -1376,6 +1380,14 @@ def test_namespaced_object_partial
get :new
assert_equal "Namespaced Partial", @response.body
end
+
+ def test_namespaced_object_partial_absolute_path
+ @controller = Quiz::QuestionsController.new
+ get :absolute
+ assert_template "_top_level_partial"
+ assert_equal "top level partial html", @response.body
+ end
+
def test_partial_collection
get :partial_collection
View
4 actionpack/test/lib/controller/fake_models.rb
@@ -56,6 +56,10 @@ def persisted?
class Store < Question
end
+
+ class Absolute < Question
+ def to_partial_path() "/top_level_partial" end
+ end
end
class Post < Struct.new(:title, :author_name, :body, :secret, :persisted, :written_on, :cost)
Something went wrong with that request. Please try again.