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

Best way to setup routes for rendering Cell::Rails cells #119

Closed
mckenziec opened this issue Apr 19, 2012 · 23 comments
Closed

Best way to setup routes for rendering Cell::Rails cells #119

mckenziec opened this issue Apr 19, 2012 · 23 comments

Comments

@mckenziec
Copy link

There is a related issue on this topic, but it doesn't address what the best practice or supported way to render a Cell::Rails cell class from routes.rb. Using Cell::Base cell class objects from routes works fine but not with Devise. If you're trying to use the Devise methods from a Cell::Base.render_cell_for initiated Cell, you'll find that the ActionController attributes are missing. e.g. like request, env, etc... This is mainly a problem if you use Devise, which wants to simply call "request" (e.g. request.env["get something"] when doing something like getting the current_user.

This works, but not with Devise methods since things like request are missing. Maybe there's a method for passing the request into render_cell_for in a way that will allow Devise to pick it up naturally.

class ActivityCell < Cell::Base
  def display(opts = {})
    puts current_user
  end

match "/dashboard/widget/:name" => proc { |env|
  cell_name = env["action_dispatch.request.path_parameters"][:name]
  [ 200, {}, [ Cell::Base.render_cell_for(cell_name, :display) ]]
}

Basically I get: NameError (undefined local variable or method `request' for #ActivityCell:0x5f51970):

If I switch to Cell::Rails:

class ActivityCell < Cell::Rails
  def display(opts = {})
    puts current_user
  end

match "/dashboard/widget/:name" => proc { |env|
  cell_name = env["action_dispatch.request.path_parameters"][:name]
  [ 200, {}, [ Cell::Base.render_cell_for(cell_name, :display) ]]
}

I get a different, familiar error: ArgumentError (wrong number of arguments (0 for 1)):

I then tried something different:

include Cells::Rails::ActionController
...
match "/dashboard/widget/:name" => proc { |env|
  cell_name = env["action_dispatch.request.path_parameters"][:name]
  [ 200, {}, [ render_cell(cell_name, :display) ]]
}

Right number of arguments, but request is still not available: NoMethodError (undefined method `request' for #ActionDispatch::Routing::Mapper:0x4c93c38)

If it's possible to instantiate Cell::Rails in the route and pass it to the render method, can someone give an example? Thank you.

@apotonick
Copy link
Member

So happy we finally address this issue. Here's the problem:

In order to make Cell::Rails feel like a real HTTPable ActionController we delegate some random methods to the parent_controller in the Cell::Rails::Metal module: https://github.com/apotonick/cells/blob/master/lib/cell/rails.rb#L10

This makes it necessary to provide the Rails cell a controller instance. Now, maybe this is not necessary in the routeable-cell setup. Maybe we don't need a controller instance there but a request object, only, which is available at the time we call render_cell_for (in env).

So, would you guys be ok with deriving your cells from, let's say, Cell::Rack and I will do the rest?

@duhmojo
Copy link

duhmojo commented Apr 19, 2012

I'm down with that. A Cell:Rack would mean we would have a clear, separate Cell to inherit from that would be likely compatible with anyother Controller gem, like Devise, we might need. For lighter Cells there's always Base and Rails.

@apotonick
Copy link
Member

Cell::Rails is pretty heavy-weighted since it carries around an ActionController instance. Cell::Rack just owns a request object so this could actually make cells work in other environments, like Sinatra, while still having a working devise API, etc. Exciting stuff!!!

@mckenziec
Copy link
Author

Honestly I'm not sure what's needed for what use cases. I think we can all agree that Cell::Rails represents an ActionController pretty well when normally rendering from a regular View. But if we're talking rendering from routes, it's missing needed rack pieces for things that rely on session and request. I'm not sure what a Cell would normally use from ActionController that would be missing when using Cell::Rack. I'm not a Rails expert, this is why I'm relying on a great gem like Cells to isolate widget MVC for clean organization. Currently I'm only interested in support for Devise and CanCan from a Cell that's been called from routes, as I know it'll work when rendering it from a normal View. I don't know what might not be available otherwise. Thanks.

@pboling
Copy link

pboling commented Apr 19, 2012

I am very interested in this as well. We are stuck on cells 3.7.x because we need the ActionController ancestor for magic things we do in our custom cell base class. If we could still have the full fledged ActionController based cells in the latest version of cells I would increase my donations to charity. ;)

@apotonick: Maybe this is already currently possible?

@mckenziec
Copy link
Author

Same here. I should have offered a bribe as well. ;-)

@apotonick
Copy link
Member

@pboling Of course, Cell::Rails still has the #parent_controller dependency and looks and feels like an ActionController. What kind of magic in your cells are you refering to? Just curious... Why don't you checkout 3.8, maybe I can help.

@mckenziec As long as Devise etc rely on the specified internal API (e.g. accessing #session) then the new Cell::Rack should work fine. Are you ready for testing?

Speaking of charity.. http://www.pledgie.com/campaigns/16068 ;-)

@mckenziec
Copy link
Author

@apotonick I thought you would never ask. :D I'll be online and working on and off over the weekend and into next week. Let me know when you have a build/branch and I'll try using it right away.

@apotonick
Copy link
Member

@mckenziec Go for it! 911436b :-)

Like

render_cell(:comments, :show, env[how the fuck do i get the request from a rack env?]

@mckenziec
Copy link
Author

I popped the changes in and started looking at how to call it from routes.rb. The request in a ActionController seems to be an ActionDispatch::Request object. In routes.rb, I think you can create it from env like so:

request = ActionDispatch::Request.new(env)

This is the way it's done in ActionController::Middleware::ActionMiddleware.call. Doing it this way will basically assign env. The rest of the class variables you see when debugging are available as methods when constructing your own. e.g. request.fullpath, etc... If this works out, you might want to change Rack to accept env, then construct DispatchRequest there to make routes.rb require one less line.

The problem I'm having now is using Cell::Rack properly. (be gentle) My route looks like this now:

match "/dashboard/widget/:name" => proc { |env|
  cell_name = env["action_dispatch.request.path_parameters"][:name]
  request = ActionDispatch::Request.new(env)
  [ 200, {}, [ render_cell(cell_name, :display, request) ]]
}

And my Cell looks like this:

class MemberLatestLoginCell < Cell::Rack
  include Devise::Controllers::Helpers
  helper_method :current_member

  def display(opts = { })
    ...
  end
end

But I'm getting: NameError (uninitialized constant Cell::Rack)

I've probably screwed up adding your changes to my project. I honestly don't know if there's a proper way to test a custom or development Gem source set. I simply found the location of rails.rb (basically C:\ruby\1.9.1\gems\cells-3.8.3\lib\cell) and added Rack to it. (the change was clear)

Let me know if I'm doing something naive please.

@apotonick
Copy link
Member

Looks good, are you still having trouble using Cell::Rack? Require the cells repo in your Gemfile! http://gembundler.com/gemfile.html

gem "cells", :git => "git://github.com/apotonick/cells.git"

@mckenziec
Copy link
Author

I already had cells in my Gemfile, even with the direct link to the git repo, nothing has changed. The version used from github is noted as 3.8.3. Isn't that the latest version I already had? (maybe you just didn't increment the version after changing it)

I've mucked about with it too much to give you a clean picture on what I'm trying, maybe you can simply point out how I'm calling it incorrectly. In my Cell:
class MemberLatestLoginCell < Cell::Rack

In routes.rb:
match "/dashboard/widget/:name" => proc { |env|
cell_name = env["action_dispatch.request.path_parameters"][:name]
#[ 200, {}, [ render_cell(cell_name, :display) ]]
request = ActionDispatch::Request.new(env)
[ 200, {}, [ Cell::Rack.render_cell(cell_name, :display, request) ]]
}

When requesting the Cell route I get:
ArgumentError (wrong number of arguments (0 for 1)):
config/routes.rb:46:in `block (3 levels) in <top (required)>'

I did a gem uninstall cells, then with the Github linked Cells gem I bundled. It still notes the version as 3.8.3.

The error is occurring on Rack:

class MemberLatestLoginCell < Cell::Rack

@apotonick
Copy link
Member

Try this:

match "/dashboard/widget/:name" => proc { |env|
  cell_name = env["action_dispatch.request.path_parameters"][:name]
  request = ActionDispatch::Request.new(env)
  [ 200, {}, [ Cell::Base.render_cell_for(cell_name, :display, request) ]]
}

@mckenziec
Copy link
Author

Found it. I don't have all the answers, but using your suggestion I still get the same Argument Error. Debug tracing deep into Rails classes can be frustrating, but I tracked the issue down to builder.rb, Cell::Builder.create_cell(*args)

    # Override this if you want to receive arguments right in the cell constructor.
    def create_cell(*args)
      new
    end

The new doesn't work for what I assume is the constructed Cell::Rack class (by name). I evaluated with new(*args) and the Cell::Rack class was constructed fine.

Is there a reason why we can't use the following in all create_cell cases? I mean the *args is passed in, I'm not sure why new can't included them.

    # Override this if you want to receive arguments right in the cell constructor.
    def create_cell(*args)
      new(*args)
    end

If you think this is a fix that'll be compatible with the three Cell types, go ahead and make the change. I'll test it out with Cell::Rake.

@mckenziec mckenziec reopened this Apr 24, 2012
@mckenziec
Copy link
Author

So how do I override Cell::Builder.create_cell? Should it be done from Cell::Rack? (Clicked the wrong button by accident! This isn't closed! I'm wired because of the hockey game)

@apotonick
Copy link
Member

I'll fix it, have to think a bit more about it! Tonight ;)

@apotonick
Copy link
Member

Check commit 2f87fdf

Your code should go like this now:

match "/dashboard/widget/:name" => proc { |env|
  cell_name = env["action_dispatch.request.path_parameters"][:name]
  request = ActionDispatch::Request.new(env)
  [ 200, {}, [ Cell::Rack.render_cell_for(cell_name, :display, request) ]]
}

@mckenziec
Copy link
Author

Bingo, that did it. Are you happy with the way the request is passed in? You could accept env as well or instead. I'm fine with either way.

@apotonick
Copy link
Member

What else information is in the env that we need in the cell? I want to keep cells decoupled from HTTP by all means..

@mckenziec
Copy link
Author

Not sure. The parts I care about thus far all seem to be in place. Devise works (because the request is present) and I can access all my Models as from a normal Controller. Will you be bumping the version to 8.3.4? I'd like to go back to a fixed version gem. Thanks!

@mckenziec
Copy link
Author

Still playing with the changes. Here's what I did to make sure any request parameters made their way into Cell::Rake. I also set the action with a default, that can be specified in the request parameters:

match "/dashboard/widget/:name" => proc { |env|
  cell_name = env["action_dispatch.request.path_parameters"][:name]
  request = ActionDispatch::Request.new(env)
  parameters = request.query_parameters
  action = parameters['action']?parameters['action'] : :display
  [ 200, {}, [ Cell::Rack.render_cell_for(cell_name, :display, request, parameters) ]]
}

@twetzel
Copy link

twetzel commented Jul 23, 2012

please change Readme to:

class PostCell < Cell::Rack
..

In your routes.rb file, mount the cell like a Rack app.

match "/posts" => proc { |env|
  request = ActionDispatch::Request.new(env)
  [ 200, {}, [ Cell::Base.render_cell_for(:post, :show, request) ]]
}

.. takes a while to find that solution

@kikorb
Copy link

kikorb commented May 19, 2015

Hi,

I think doing this

match "/dashboard/widget/:name" => proc { |env|
  cell_name = env["action_dispatch.request.path_parameters"][:name]
  request = ActionDispatch::Request.new(env)
  [ 200, {}, [ Cell::Rack.render_cell_for(cell_name, :display, request) ]]
}

No longer works since the third param is actually expecting a controller and not a request object (In fact you can get undefined method request for request object)

The solution I found is to do this:

match "/posts" => proc { |env|
  controller = ActionController::Base.new
  controller.request = ActionDispatch::Request.new(env)

  [ 200, {}, [ Cell::Rack.render_cell_for(:post, :show, controller, controller.request.query_parameters) ]]
}

By doing this your cell can be a Cell::Rails and you will be able to mount it in the routes.
I have writed about this here

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

6 participants