Presenting more specifically when using modules

[terlar]

I think it is kind of nice that Some::Name::Space falls back to the presenter SpacePresenter. But if I had some more specific presenter, say Some::Name::SpacePresenter it doesn't seem to recognise it.

I get ther error:
uninitialized constant Some::Name::SpacePresenter

If I had the SpacePresenter it would use that one even if I had the more specific Some::Name::SpacePresenter

[felipeelias]

Maybe I didn't fully comprehend the question, but if you have a namespaced class, it will attempt to get the namespaced presenter, check out the code here

Maybe there are some loading issues?

[terlar]

Yes, it seems the less specific one is used even if you have specified a more specific one. Sorry about the description, it seems I had two errors in one, the uninitialized was because my loading issues, they are solved now.

I have a LocationPresenter and a Search::Linkshelf::LocationPresenter. If I have both these the first one will be used for the Search::Linkshelf::Location objects. If I remove the first one the more specific one is used. As intended.

[felipeelias]

Ok now I see your problem. I will create a test case for that and check what's happening. Thanks!

[terlar]

I'm having a hard time reproducing this in an isolated environment. Will see if I can isolate the issue and then it should be easier to make a test case for it.

[felipeelias]

I'm just guessing, but are you presenting something like this?

class Namespace::Controller
  def index
    present Model.new # which points to Namespace::Model
  end
end

[terlar]

No, unfortunately not. I have succeeded to narrow down the scope some more. I have setup a dummy rails project because that is the only place I have been able to reproduce it. The strange thing is it only breaks in the access of the page, not in tests.

This is basically what I did to reproduce this:
config/routes.rb

TestResubject::Application.routes.draw do
  get :box, to: 'box#index'
end

app/controllers/box_controller.rb

class BoxController < ActionController::Base
  def index
    @box = present Box.new
    @fancy_box = present Fancy::Box.new
    Rails.logger.info @box.class.name
    Rails.logger.info @fancy_box.class.name
  end
end

app/views/box/index.html.erb

<ul>
  <li><%= @box.contents %></li>
  <li><%= @fancy_box.contents %></li>
</ul>

app/models/box.rb

class Box
end

app/models/fancy/box.rb

class Fancy::Box
end

app/presenters/box_presenter.rb

class BoxPresenter < Resubject::Presenter
  def contents
    "#{self.class.name} presenting #{to_model.class.name}"
  end
end

app/presenters/fancy/box_presenter.rb

class Fancy::BoxPresenter < Resubject::Presenter
  def contents
    "#{self.class.name} presenting #{to_model.class.name}"
  end
end

spec/controllers/box_controller_spec.rb

require 'spec_helper'
describe BoxController do
  it 'works' do
    get :index
    expect(assigns(:box)).to be_a BoxPresenter
    expect(assigns(:fancy_box)).to be_a Fancy::BoxPresenter
  end
end

[terlar]

The strange thing is this passes in the tests with correct presenters.

But when visiting the page in the browser I get:

BoxPresenter presenting Box
BoxPresenter presenting Fancy::Box

and the log file shows:

BoxPresenter
BoxPresenter

However if I switch place like this in the controller action:

@fancy_box = present Fancy::Box.new
@box = present Box.new

Then it works! Insanely strange IMO.

[felipeelias]

Try this:

class BoxController < ActionController::Base
  require_dependency 'app/presenters/fancy/box_presenter'

  def index
    @box = present Box.new
    @fancy_box = present Fancy::Box.new
    Rails.logger.info @box.class.name
    Rails.logger.info @fancy_box.class.name
  end
end

[felipeelias]

This is clearly an autoload issue. Basically on Naming class, if we replace:

presenter.split('::').inject(Object) { |ns, cons| ns.const_get(cons) }

with

presenter.constantize

It works.

The problem is with Rails autoload. Since the module Fancy is auto-created (it's not defined anywhere), when you call Fancy::Box, rails loads fancy/box.rb. For some reason, and I don't know why, when you run this code after presenting Fancy::Box:

Object.const_get('Fancy').const_get('BoxPresenter')
=> BoxPresenter

It returns the wrong constant. However if you run this at the top of your action, it works fine, because it triggers the load of Fancy module from Box presenter.

[terlar]

I tried the require_dependency solution and it worked. So I guess this might be a bug with Rails then? However I solved it by renaming my classes which in some cases wouldn't be the right thing to do.

How do we proceed from here?

[felipeelias]

It's not a bug in Rails, it's just how it works - if you have this exact situation - and you have to rely on require_dependency.

However there's a fix for the Naming class that I mentioned before that solves the issue, which forces Rails to load the correct constant. The const_get method from ruby does not trigger the autoload, which is pretty much the expected behaviour.

One clarification: since BoxPresenter is defined in the top-level namespace and the namespaced one is not available (loaded by rails), the ruby's constant lookup finds the top-level one.

[terlar]

I just found out that this issue can be solved by doing the following.

presenter.split('::').inject(Object) { |ns, cons| ns.const_get(cons, false) }

http://ruby-doc.org/core-1.9.3/Module.html#method-i-const_get

I am not sure if this is the desired behaviour in all cases. All the tests are passing with this change though, so I guess it is not a intended behavior?

[felipeelias]

wow, nice finding!

I just tested and you're right, it fixes this scenario with rails autoload.

I couldn't write a failing test to justify this thought... Unless I require ActiveSupport and create a very specific scenario (which I think it doesn't make sense).

If you have this on your fork, could you please open a PR?

I'd just comment out the code on why this decision was taken, since we can't justify writing tests.

Thanks again!