trailblazer / cells

View components for Ruby and Rails.
https://trailblazer.to/2.1/docs/cells.html
3.07k stars 236 forks source link

context doesn't seem to work with collections #413

Closed samstickland closed 7 years ago

samstickland commented 8 years ago

Hi,

context only seems to get passed to the first loop when rendering a cell with a collection.

I call the cell like this:

cell('work_histories/list_entry', collection: @work_histories, context: { current_user: current_user })

In the first collection entry context is set correctly:

[10, 19] in /home/sam/hubbado/hubbadoproto/app/cells/work_histories/list_entry_cell.rb
   10: 
   11:   delegate :dates, to: :decorated
   12: 
   13:   def show
   14:     debugger
=> 15:     render
   16:   end
   17: 
   18:   def incomplete
   19:     debugger
(byebug) @options[:context].keys
[:current_user, :controller]

But in the second invocation it's current_user is gone:

[10, 19] in /home/sam/hubbado/hubbadoproto/app/cells/work_histories/list_entry_cell.rb
   10: 
   11:   delegate :dates, to: :decorated
   12: 
   13:   def show
   14:     debugger
=> 15:     render
   16:   end
   17: 
   18:   def incomplete
   19:     debugger
(byebug) @options[:context].keys
[:controller]
samstickland commented 8 years ago

OK, I found the problem.. That was a red herring above.

The problem is that for a collection the options all end up in model, but in cells-rails options is initialised in the rails helper:

    options[:context] ||= {}
    options[:context][:controller] = self

Then in cells there's this bit of code in Cells (lib/cell/view_model.rb) where it merges the options for a collection

  def call(model=nil, options={}, &block)
    if model.is_a?(Hash) and array = model[:collection]
      return Collection.new(array, model.merge(options), self)
    end

    build(model, options)
  end

The problem is that both model and options contain a context key so one of them gets overwritten, rather than merged.

A possible fix would be this:

    def call(model=nil, options={}, &block)
      if model.is_a?(Hash) and array = model[:collection]
        merged_context = (model[:context] || {}).merge(options[:context])

        merged_options = model.merge(options)
        merged_options[:context] = merged_context

        return Collection.new(array, merged_options, self)
      end

      build(model, options)
    end
samstickland commented 8 years ago

https://github.com/samstickland/cells/commit/1ff3eac397ec87344cb1bdcafb1365c21dab1fa6

Sorry, I won't have time to write a test for this until next week.

apotonick commented 7 years ago

This is tested and fixed.

samstickland commented 7 years ago

@apotonick : Hi Nick, which release can I find this fix in?

apotonick commented 7 years ago

Latest Cells!