#extends should raise error if no template found #192
Comments
|
Good point, perhaps this can be a configuration option. |
|
I believe I am encountering this issue at the moment. I have a "products/index" template and a "products/show" template which both contain the same attributes. When I attempt to use the show template in the index template to replace the attributes line with Raising an exception would be excellent, and telling us where Rabl is looking for these templates would be even better. |
|
Yep, fair points. Since spree is open-source, I will take a look and try to figure out why it isn't working. If you are curious though, the template path is resolved like this: https://github.com/nesquena/rabl/blob/master/lib/rabl/partials.rb#L40 |
|
The tests inside the spec directory (particularly spec/controllers/api/v1/products_controller_spec) should aid in the debugging. Thanks so much for taking a look, Nathan! On Monday, 26 March 2012 at 1:30 PM, Nathan Esquenazi wrote:
|
|
Thanks, will dig into this sometime today or tomorrow and report back here. |
|
I've dug into this problem this afternoon and it looks like it's happening because Hope that helps! |
|
Should |
|
@radar Yes that code path was a result of me integrating a patch for Rails template lookup. I am actually not very familiar with the correct way to leverage Rails for finding me a template. If you notice though, the second code path (in the else) is the way I used to go about it. Let me ask it this way. Is there a simple way to tweak this such that Rails 3 and Rails 2 would support the usage of if defined?(@_scope) && @_scope.respond_to?(:find_template)
# use Rails's own template resolution mechanism (partials and no partial)
lookup_proc = lambda { |partial| @_scope.find_template(file, [], partial) }
template = lookup_proc.call(false) rescue lookup_proc.call(true)
file_path = File.join(Rails.root.to_s, template.inspect) if template
else # fallback to manual
root_path = Rails.root
view_path = options[:view_path] || File.join(root_path, "app/views/")
file_path = Dir[File.join(view_path, file + ".{*.,}rabl")].first
endIt looks like you are suggesting: if defined?(@_scope) && @_scope.respond_to?(:lookup_context)
# use Rails's own template resolution mechanism (partials and no partial)
lookup_proc = lambda { |partial| @_scope.lookup_context.find_template(file, [], partial) }
template = lookup_proc.call(false) rescue lookup_proc.call(true)
file_path = File.join(Rails.root.to_s, template.inspect) if template
else # fallback to manual
root_path = Rails.root
view_path = options[:view_path] || File.join(root_path, "app/views/")
file_path = Dir[File.join(view_path, file + ".{*.,}rabl")].first
endWould that work in just Rails 3+ I am assuming? Is there an equivalent in Rails 2.3? Looks like http://apidock.com/rails/ActionView/PathSet/find_template suggests something like: @_scope.view_paths.find_template(...) could work in Rails 2.3? Considering you wrote "Rails 3 in action", I am going to assume you have a pretty solid handle on all this :) Would love your guidance on making the template resolution for Rails 2 and 3 as robust as possible |
|
Also keep in mind I have Rails 2 and Rails 3 full integration testing that I run with each rabl release including full testing of the use of extends and partial. So in at least the most trivial case, I am assuming the 'fallback' case must be working. However, perhaps the first code path (find_template) doesn't work or not with the later versions of Rails. |
|
Also, sorry to keep posting but I just tested the current code path in Rails 3 and at least there the existing |
|
@radar OK, quick update. Added Rails 3.2 template integration testing, and tweaked template resolution here: f1fbe39 to use lookup_context. Also I raise now if no template is found. Tested this in Sinatra, Padrino, Rails 2+3+3.2 integration tests and everything seemed to pass. Can you try rabl git master on your project and let me know if it works any better? |
|
@wulftone can you try rabl from git and verify that it raises as expected now? |
|
@radar Had a bit more time. Forked spree and fixed all the tests: nesquena/spree@4057b25 . Also confirmed that raising works now. All released into 0.6.3. Closing. Fun side note, you need to use |
|
The new partial finding code looks a lot cleaner! Nice work. |
|
Indeed much cleaner and supports Rails 2, 3, and 3.2. Was definitely worth digging into. Thanks for bringing this up. |
|
Hmm it still fails silently on a call to No error raised! |
|
I am confused, why wouldn't Line 59 in f1fbe39 |
|
Ah yes, because I'm a moron! I did the above test in a console that was already running. : \ Sorry, when I first looked at your solution I was like "Yes!" and then I fooled myself into over analyzing why it failed in my test without first checking the obvious. It works as advertised. : P Thank you! |
|
Ha OK thanks for following up, I appreciate it. Glad to hear everything is working as expected now. Let me know if you run into any other issues. |
|
Maybe I spoke too soon... It's been causing some of my integration specs to fail... Here's some console tests I did while changing the Gemfile to require rabl 0.6.2 (which works) and 0.6.5 (which fails)... complete with bundle updates and restarting the console! : P With rabl 0.6.5: And after a Here's my rabl template: In 0.6.5 it raises the error as expected in the console, but shows up as this cryptic error in my capybara/webkit integration tests: It fails in both test and dev environments on 0.6.5, and works fine in 0.6.2. |
|
fwiw, I still cannot use extends within a child block either and I've worked around it by doing this: https://github.com/spree/spree/blob/api/api/app/views/spree/api/v1/line_items/show.rabl#L3-5 |
|
Hmm. I haven't had a problem using extends within a child block. It was the |
|
Ah, I was being stupid. |
|
@radar Does that mean everything is working ok then? |
|
If you're still keen to investigate something, I suspect I may not be able to use |
|
Really? In my original pull request https://github.com/nesquena/spree/blob/a698a3ffc4f0f3f93351cc177229cc8a2f1e314d/api/app/views/spree/api/v1/line_items/show.rabl#L4 I had changed it to using extends and the tests seemed to pass without error. This was with 0.6.5. I can take another look though today. Out of curiosity if you change it to |
|
Yeah, was getting a non-"rails can't find this template" error for the template this morning, indicating that it's falling right to the end of that method again. |
|
Cool I'll take a look. This Rails template resolution logic is starting to get out of hand how many edge cases there seem to be haha. |
|
Did you take the lunar calendar into consideration when building the template resolution? ;) |
|
Ha, it's getting to that point. If I can fix these last issues here and we can work through #202 then I'm hoping at that point resolution will be fairly reliable. |
|
@radar OK, I think your issue is you don't have Check spree/spree#1337 that makes the tests pass |
|
@wulftone Can you try from master? I think I fixed it in a pretty easy way. (when there's no template, fall back to manual lookup) |
|
I tried switching the extensions like so: So, using style 3, my rabl file looks like this: In 0.6.2, style 1 works. Rails, why do you make things so difficult sometimes? : P |
|
OK, we have a patch here: #205 (comment) that may address this. I am going to integrate it and ask if you can test it again. Sometimes the complexity of this Rails template handling does start to get frustrating though haha. |
|
Ok I think this is working now... although it still raises the error you added to partials.rb in the rails console (using the method of calling the template I posted above), it seems to work on the development and test servers. Thanks a ton for doing this, I know it wasn't the most painless process ever... : ) |
|
Awesome, thanks for testing again. Let me know if you run into any other issues. |
extends should raise error if no template found... it would be nice if it interrupted execution and told us what was wrong. A typo or whatever could be hard to track down otherwise.
The text was updated successfully, but these errors were encountered: