trailblazer / cells

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

Problem with output_buffer in tests #298

Closed opti closed 9 years ago

opti commented 9 years ago

Hey Nick,

I'm not sure that it's related to cells itself, most likely this needs to be addressed to cells-erb or erbse, but posting it here as more centralized place.

I've run into an issue with specs. All crashed specs render cells with form_tag or form_for tag in a view. Crashed with the following error:

Failure/Error: subject { concept("checkout/coupon/cell", order).call }
NoMethodError:
  undefined method `output_buffer=' for #<Checkout::Coupon::Cell:0x007fefea19d778>
  # /gems/actionview-4.2.2/lib/action_view/helpers/capture_helper.rb:203:in `ensure in with_output_buffer'
  # /gems/actionview-4.2.2/lib/action_view/helpers/capture_helper.rb:203:in `with_output_buffer'
  # /gems/actionview-4.2.2/lib/action_view/helpers/capture_helper.rb:38:in `capture'
  # /gems/actionview-4.2.2/lib/action_view/helpers/form_tag_helper.rb:70:in `form_tag'
  # app/concepts/checkout/coupon/views/new.erb:6:in `block in singleton class'
  # app/concepts/checkout/coupon/views/new.erb:-1:in `instance_eval'
  # app/concepts/checkout/coupon/views/new.erb:-1:in `singleton class'
  # app/concepts/checkout/coupon/views/new.erb:-3:in `__tilt_70334275804140'
  # /gems/tilt-1.4.1/lib/tilt/template.rb:170:in `call'
  # /gems/tilt-1.4.1/lib/tilt/template.rb:170:in `evaluate'
  # /gems/tilt-1.4.1/lib/tilt/template.rb:103:in `render'
  # /gems/cells-4.0.1/lib/cell/view_model.rb:138:in `render_template'
  # /gems/cells-erb-0.0.5/lib/cell/erb/template.rb:10:in `render_template'
  # /gems/cells-4.0.1/lib/cell/view_model.rb:119:in `render_to_string'
  # /gems/cells-4.0.1/lib/cell/view_model.rb:112:in `render'
  # ./app/concepts/checkout/coupon/cell.rb:11:in `show'
  # /gems/cells-4.0.1/lib/cell/view_model.rb:126:in `render_state'
  # /gems/cells-4.0.1/lib/cell/caching.rb:47:in `render_state'
  # /gems/cells-4.0.1/lib/cell/view_model.rb:103:in `call'
  # /gems/cells-4.0.1/lib/cell/rails.rb:47:in `call'
  # /gems/cells-4.0.1/lib/cell/testing.rb:39:in `call'
  # ./spec/concepts/checkout/coupon_cell_spec.rb:8:in `block (2 levels) in <module:Coupon>'
  # ./spec/concepts/checkout/coupon_cell_spec.rb:20:in `block (4 levels) in <module:Coupon>'

I saw you discussion in #294.

rails 4.2.2 cells 4.0.1 cells-erb 0.0.5

apotonick commented 9 years ago

It looks like Cell::Erb is not included in the cells that fail, since it uses capture from Rails and not the patched one from Cell::Erb.

In your specs, can you do Cell::Concept < Cell::Erb? The result has to be true, otherwise your initialization hasn't worked properly. You can try using rspec-cells.

opti commented 9 years ago

I'm using rspec-cells and Cell::Concept < Cell::Erb returns true in my specs.

In the backtrace cells-erb-0.0.5 is also present.

  # /gems/cells-erb-0.0.5/lib/cell/erb/template.rb:10:in `render_template'
opti commented 9 years ago

However, the problem fixes when I manually include Cell::Erb in a cell (not spec).

Isn't it should be done automatically in rails env?

apotonick commented 9 years ago

Yes! In Rails, this is done automatically. Are you not working in a Rails environment? :heart:

Come join us on #trailblazer on IRC (Freenode), we can solve it there faster.

apotonick commented 9 years ago

I hate Rails helpers. It looks like it is done automatically (otherwise the Cell::Concept < Cell::Erb would be false, but maybe one of the helpers you include overrides capture? I hate Rails helpers. Can I see the failing cell code? I hate Rails helpers.

opti commented 9 years ago

Here it is: concepts/checkout/coupon/cell.rb

module Checkout
  module Coupon
    class Cell < Cell::Concept
      include ActionView::Helpers::UrlHelper
      include ActionView::Helpers::FormTagHelper
      include ::Cell::Erb

      property :coupon
      property :number

      def show
        coupon ? render(:current) : render(:new)
      end

      private

      def coupon_code
        coupon.code
      end

      def coupons_path
        checkout_coupons_path(number)
      end

      def contract
        options[:contract]
      end
    end
  end
end

P.S. I can fill you pain about rails helpers

apotonick commented 9 years ago

Does this cell work in a view?

Please, join me on IRC, I can help you much quicker there.

opti commented 9 years ago

As it turned out, the problem was in not necessary inclusion of rails helpers in the cell:

include ActionView::Helpers::UrlHelper
include ActionView::Helpers::FormTagHelper

Those two are already included by Cells, so don't need to do it second time, because it overrides cells' capture implementation.

So, the correct version looks like that:

module Checkout
  module Coupon
    class Cell < Cell::Concept
      property :coupon
      property :number

      def show
        coupon ? render(:current) : render(:new)
      end

      ...

    end
  end
end
tjjjwxzq commented 7 years ago

Hmmm...I'm getting a similar problem but when I pass a block to a link_to helper in my view (if I don't pass a block it seems fine). This only breaks the specs, but it's fine in a real environment. Using Cells 4.1.5, Cells-Rails 0.0.6, and Cells-Slim 0.0.5

My cell class

class EventProposal::IndexCell < Cell::ViewModel
    # I have to manually include this stuff for my specs to pass
    # even though it's supposed to be already handled by cells-rails
    include ActionView::Helpers::FormHelper
    include Cell::RailsExtensions::HelpersAreShit
    include Cell::Slim

    def show
        render
    end

And when the specs break, this is what I get

EventProposal::IndexCell#show when the current user is an osl admin 
      Failure/Error: = link_to '' do

      NoMethodError:
        undefined method `output_buffer=' for #<EventProposal::IndexCell:0x0055c0836bedd0>
        Did you mean?  output_buffer
      # app/cells/event_proposal/index/show.slim:1:in `block in singleton class'
      # app/cells/event_proposal/index/show.slim:-2:in `instance_eval'
      # app/cells/event_proposal/index/show.slim:-2:in `singleton class'
      # app/cells/event_proposal/index/show.slim:-4:in `__tilt_47142621548820'
      # ./app/cells/event_proposal/index_cell.rb:16:in `show'
      # ./spec/cells/event_proposal/index_cell_spec.rb:98:in `block (4 levels) in <top (required)>'
      # ./spec/cells/event_proposal/index_cell_spec.rb:102:in `block (4 levels) in <top (required)>'
      # ------------------
      # --- Caused by: ---
      # NoMethodError:
      #   undefined method `output_buffer=' for #<EventProposal::IndexCell:0x0055c0836bedd0>
      #   Did you mean?  output_buffer
      #   app/cells/event_proposal/index/show.slim:1:in `block in singleton class'

So I realize Cell::Slim::Rails::Helpers is supposed to override Rails' shitty capture helper, but for some reason the order of the module inclusion in the testing environment is such that I get ActionView::Helpers::CaptureHelper before Cell::Slim::Rails::Helpers in the list of included modules. So I'm basically getting this:

# real environment, inside the template
= raise self.class.included_modules.inspect
[... Cell::Slim::Rails::Helpers, ...ActionView::Helpers::CaptureHelper, ...]

# test environment, inside the template
= raise self.class.included_modules.inspect
[...ActionView::Helpers::CaptureHelper, ...Cell::Slim::Rails::Helpers, ]

Quick and dirty fix is to do this in my specs:

before do
   described_class.class_eval do
      prepend Cell::Slim
      prepend Cell::RailsExtensions::HelpersAreShit
   end
end

but I hope you can shed light on why the module inclusion order is different in the test environment? (I should add also that this seems to only happen after I add in config/application.rb a line to specify cells with assets config.cells.with_assets = ['event_proposal/index_cell']. Everything was fine and dandy before that)

(thanks for opening my eyes to the wonderful mess that is the Rails helper system)

tjjjwxzq commented 7 years ago

Actually, this doesn't seem to be determined by environment, and seems to depend on where the cell is initialized and/or stuff I add to config, presumably because the initialization (and hence helper module inclusion order) is modified. Since we must have Cell::RailsExtensions::HelpersAreShit and Cell::Slim overriding the default rails helpers, can we not have the cells-rails railtie prepend these modules instead of simply including them?

apotonick commented 7 years ago

If you find the correct position in the Railtie initializer chain, I'm happy to merge it in cells-rails. :+1:

george-carlin commented 7 years ago

Just noting that I'm getting the same error in tests for a cell that includes ActionView::Helpers::FormOptionsHelper (so I can use options_for_select). Everything works as expected when testing in the browser, but RSpec fails with undefined methodoutput_buffer=' for #`.

Don't have time to investigate it properly, but my hacky fix for now is to include ::Cell::Erb in my Cell, below the line include ActionView::Helpers::FormOptionsHelper.