Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#controller is available only for the first model in a collection #300

Open
BlackFoks opened this issue Jun 24, 2015 · 15 comments
Open

#controller is available only for the first model in a collection #300

BlackFoks opened this issue Jun 24, 2015 · 15 comments

Comments

@BlackFoks
Copy link

When rendering a collection #controller is available only for the first model in a collection.

Let's we want to render a list of books:

books = [Book.find(1), Book.find(2), Book.find(3)]

Then #controller will be available only for Book#1, when rendering Book#2:

def show
  controller # => nil
end

I don't think it's an expected behaviour. I found what caused this https://github.com/apotonick/cells/blob/f785b3431b226b5c98a5cd58d4fc76e13239f0d7/lib/cell/twin.rb#L17

super(build_twin(model, options), controller: options.delete(:controller))

We delete :controller from options. I see 2 possible ways to fix this:

  1. Don't options.delete(:controller), just do options[:controller]
  2. When rendering collection do options.dup or something like this.

I can send a PR if you tell me what's the best way. I'd like 1st but I'm not sure about consequences (maybe it's a required action or just a try to hide controller from twin).

@apotonick
Copy link
Member

Twin is not supported in Cells 4 (yet) - why do you have that included?

@BlackFoks
Copy link
Author

I have a twin class to share some code between different representers. I just want to use its properties in a cell (concept actually).

@apotonick
Copy link
Member

Hm, when you copy it, it should work. I will re-add twin support when I release Disposable.

@apotonick
Copy link
Member

It works without twin, here's the proof: https://github.com/apotonick/cells/blob/master/test/public_test.rb#L35 😜

Can you paste some code, I'm curious to know how you use twins?

@BlackFoks
Copy link
Author

Yes, it works without twin. I just tested it. Ok, here is some code:

class Invoice < ActiveRecord::Base
  # ...
end

class Invoice::Twin < BaseTwin
  # properties here...

  module XeroIntegration
    def invoice_number
      "INV-#{ id }"
    end

    def xero_url
      if bill?
        xero_bill_url
      else
        xero_invoice_url
      end
    end

    protected

    def xero_bill_url
      # ...
    end

    def xero_invoice_url
      # ...
    end
  end
  include XeroIntegration
end

class Invoice::Concept < Concept
  include ::Cell::Twin

  def show
    #...
  end

  private

  twin ::Invoice::Twin
end

class Invoice::XeroRepresenter
  def initialize(invoice)
    @invoice = ::Invoice::Twin.new(invoice)
  end

  def to_hash
    {
      number: invoice.invoice_number,
      # other properties
    }
  end
end

So I use twin as a decorator. I didn't want to put twin's code into a model so I though it was a good idea to use twins for this.

@apotonick
Copy link
Member

Yeah, that's exactly how it's meant to work. 😬 You should use a representer to compile your to_hash hash with Representable. You can infer a representer from a twin.

@BlackFoks
Copy link
Author

Yes, I can I know. It's not an issue :) The issue is that including include ::Cell::Twin into Invoice::Concept breaks collection rendering.

@BlackFoks
Copy link
Author

I just updated cells to 4.0.1 and it broke device helpers in cells. I already read that helper_method is not available anymore so I tried to use something like delegate :current_user, to: :controller to get a current user inside a cell. It works well expect collections because controller is nil for all non-first elements.

@apotonick
Copy link
Member

But it only breaks with Twin included, right?

@BlackFoks
Copy link
Author

Yes, I think it's due to options.delete(:controller) and then options is passed to the next element with no :controller in options.

@BlackFoks
Copy link
Author

It's here https://github.com/apotonick/cells/blob/f785b3431b226b5c98a5cd58d4fc76e13239f0d7/lib/cell/twin.rb#L17

So it builds the first element with controller and then controller gets lost.

@apotonick
Copy link
Member

I know, I know, I have to rewrite Twin for cells anyway.

@BlackFoks
Copy link
Author

Ok, thanks again.

Just one thought: since anyone can modify options in one cell and it will affect all next cells in a collection maybe it would be good to pass a copy of original options to each cell to prevent such issues in future?

@apotonick
Copy link
Member

This will dramatically slow down rendering collections! Hash operations are expensive!

@BlackFoks
Copy link
Author

Oh, I see.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants