Establish working pattern for rendering partials from an exhibit with a freshly exhibited object.

[pdobb]

The need for this isn't easy to explain... and this pull request isn't necessarily what's needed (maybe there's an opportunity to automatically "reexhibit" objects). But this pull request is at least one way to present the issue I've run into along with the quick/easy solution that's been working for me for the past 6 months each time I run into it: the DisplayCase::Exhibit#reexhibit method.

By code example:

  # Given:
  class MyExhibit1 < DisplayCase::Exhibit
    def self.applicable_to?(*); true end

    def foo; "foo" end

    def do_render_from_my_exhibit(context)
      context.render "my_partial", my_local_var: self
    end
  end

  class MyExhibit2 < DisplayCase::Exhibit
    def self.applicable_to?(*); true end

    def bar; "bar" end
  end

  # When executed from console, works as expected:
  ex_obj = DisplayCase::Exhibit(my_obj)
  ex_obj.foo # => "foo"
  ex_obj.bar # => "bar"

  # When executed through a render:
  #1) From within a view template
  @ex_obj.do_render_from_my_exhibit(self)

  #2) From within "my_partial" view template
  my_local_var.foo # => "foo"
  my_local_var.bar # => Undefined method `#bar`!
  # `my_local_var` seems to be "frozen" as a MyExhibit1

  # HOWEVER

  # If we send a "reexhibited" version of self as a local var:
  class MyExhibit1 < DisplayCase::Exhibit
    def do_render_from_my_exhibit(context)
      context.render "my_partial", my_local_var: reexhibit
    end
  end

  # Then:
  # When executed through a render:
  #1) From within a view template
  @ex_obj.do_render_from_my_exhibit(self)

  #2) From within "my_partial" view template
  my_local_var.foo # => "foo"
  my_local_var.bar # => "bar"
  # This works! my_local_var is now able to act like a regular exhibit again.

I hope this makes sense. The essence of it is that after performing a render call, the objects being passed through on the locals hash no longer behave as exhibits should. That is, normally exhibited objects find the first exhibited object that can respond to a method. But after going through a render they only respond to methods on the exhibited object that was passed in, unless reexhibited first.

Also, I was trying to write tests for this example and was having trouble again. Trying to get an exhibited object to go through a render call seems like it would require adding a development dependency on Rails and I wasn't sure how to approach it even. And mocking didn't seem able to reproduce the issue.

It desired, I'll try to set up a simplest case working code example... let me know.

I also started a CHANGELONG.md file for the project, so if nothing else maybe you'd like to take that part away from this :).

[codeodor]

👍 on the changelog.

I hate to ask, but will you do that minimal working code sample?

If we can, I would rather fix it in the general sense rather than have a re-exhibit. If not, then we have to provide some workaround, so this would be it.

Don't rush on the sample code -- I won't be able to work on it until next week. And then I'll fork your repo and send you a PR if I can find a solution, which we can then merge into this one.

[pdobb]

Thanks @codeodor. I made the test app showing the issue and the workaround. I'm going to take a stab at an actual fix (that doesn't use the reexhibit workaround) later, too.

[codeodor]

That would be fantastic. I'll devote a little time next week as well. Thank you!

[pdobb]

After going over it for a while, it seems to me there's little we can do to fully protect this bug... but first let me explain what I found. Basically, when you call a method on an exhibited object, then self, as evaluated from inside the method in the Exhibit object, loses the rest of the exhibit chain. So self thinks of itself as e.g. OneExhibit:DisplayCase::BasicExhibit(DisplayCase::BasicExhibit((#<User id: 1, created_at: "2014-09-04 21:29:07", updated_at: "2014-09-04 21:29:07">))) instead of TwoExhibit:OneExhibit:DisplayCase::BasicExhibit(OneExhibit:DisplayCase::BasicExhibit(DisplayCase::BasicExhibit((#<User id: 1, created_at: "2014-09-04 21:29:07", updated_at: "2014-09-04 21:29:07">)))) -- as it was just before going into the Exhibit method. This is the key "issue", but I'm assuming that this is also just how it is / has to be.

Now, since the user is free to bypass the DisplayCase::Exhibit#render method (which I'd always done -- see do_render_with_self1 from the test app) I don't think there's anything we can do other than add a note in the README on how to use the reexhibit method I've provided. But if the users utilizes DisplayCase::Exhibit#render to render partials from within an exhibit (see do_render_with_self2 from the test app), we could save them the trouble by "reexhibiting" self for them. I.e. change

def render(template, options = {})
  template.render(options.reverse_merge(:partial => to_partial_path, :object => self))
end

to:

def render(template, options = {})
  template.render(options.reverse_merge(:partial => to_partial_path, :object => reexhibit))
end

This solves the original problem by ensuring that the object within the partial has the full exhibit chain and is not narrowed down to just the Exhibit type that was just rendered.

I'm going to pause my work on this here to give you time to catch up next week. Maybe you can invalidate my original assumption on the key "issue" here. Thanks again and see you next week!

[codeodor]

No, I think you're right. But here's what I'd like to see as a fix, if you think it will still work:

1. No need for reexhibit method since we can just call exhibit again.
2. change render in DisplayCase as you mentioned above, but just call exhibit instead of reexhibit
3. update the readme examples to show that we should call render on the exhibit itself instead of calling it on the template/context
4. make a note in the readme in case you call render on the context, you'll need to exhibit self. (perhaps even linking to this issue)

Thoughts?

[pdobb]

Can't just call exhibit by itself because DisplayCase::Exhibit#exhibit requires 1 argument. And exhibit(self) doesn't successfully reexhibit the object, it just returns the already exhibited object, pretty much. It requires exhibit(to_model) to get a "full", new exhibit of the original object and solve the issue. So it's either we use/require that full notation or we add a method to take care of that responsibility; and I vote method for ease of use. But if "reexhibit" isn't the best name we can brainstorm, by all means.

The rest sounds great and I'm happy to take a crack at updating the README, etc.

[codeodor]

Sorry, I did mean that we'd call exhibit(to_model)

[codeodor]

I meant to thank you for the test app too. That was freaking fantastic and made it so easy to follow along. I really appreciate that.

[pdobb]

Ahh, OK. So I'll make the changes to the pull request as per your outline.

You bet, the test app was a little extra work, but I agree... it clarified the whole issue for me as well :).

[pdobb]

Alrighty, there it is. My wording may not be the clearest so please fix up as you see fit or let me know if I can contribute anything more. Cheers!

[codeodor]

Thanks @pdobb I'll see about fixing up the build before merging.

[codeodor]

Thanks a ton for your help @pdobb!

I also released version 0.1.1 up to RubyGems, hurray! 🎆

[pdobb]

👍 Thanks! Don't forget to change the "Unreleased" header in the changelog to v0.1.1 then, too :)

[codeodor]

OOOPS, thanks for reminding me!