Tell me more ×
Stack Overflow is a question and answer site for professional and enthusiast programmers. It's 100% free, no registration required.
up vote 1 down vote favorite

Hey guys I need your valuable help. In the Ruby on Rails tutorial, at the 10th chapter, the author has 2 controllers in one view. I build a similar app in which I also have 2 controllers in one view, however, when I render a method, which belongs to the second controller, from the first I have problems with the params. (in the tutorial the author does not use any parameters in the action of the other controller)

More specifically I have 2 controllers: UsersController and MicropostsController. Also, in the show.html.haml page of Users, I use both controllers: UsersController for showing user's microposts and MicropostsController in order to allow the user to create a new micropost.

Inside MicropostsController:

def create
  @micropost = current_user.microposts.build(params[:micropost])
  if @micropost.save
    flash[:success] = "Micropost created!"
    redirect_to user_path(current_user)
  else
    #render text: renderActionInOtherController(UsersController,:show, {:id => 1})
    @user = User.find(current_user)
    @microposts = @user.microposts.paginate(page: params[:page])
    render 'users/show'
  end
end

Inside UsersController

def show
  @user = User.find(params[:id])
  @microposts = @user.microposts.paginate(page: params[:page])
  @micropost = current_user.microposts.build if signed_in?
end

In the app/views/users/show.html.haml

- provide(:title, @user.name)
.users_page
  .row
    %aside.span4
      - if !signed_in?
        %section
          %h1
            = gravatar_for @user
            = @user.name
      - else
        %section
          = render 'shared/user_info'
        %section
          = render 'shared/micropost_form'

    .span8
      - if @user.microposts.any?
        %h3 Microposts (#{@user.microposts.count})
        %ol.microposts
          = render @microposts
        = will_paginate @microposts

So basically my question summarizes as follows:

1)Is it a good practice to have multiple controllers in 1 view? I have found contradicting answers in the net. (actually, I am not even sure if this code remains RESTful)

2) If 1 is yes (or at least it is not a bad practice) could I implement the same thing in a more efficient way? Because the way I see it, every time I render action from another controller, I have to redefine the variables.

3) I found a similar topic in stackoverflow in which one proposes to use this method (which I don't get what it does exactly since I am new in RoR).

def renderActionInOtherController(controller,action,params)
  controller.class_eval{
    def params=(params); @params = params end
    def params; @params end
  }
  c = controller.new
  c.request = @_request
  c.response = @_response
  c.params = params
  c.send(action)
  c.response.body
end

If I use this version of create action inside MicropostsController, 

def create
  @micropost = current_user.microposts.build(params[:micropost])
  if @micropost.save
    flash[:success] = "Micropost created!"
    redirect_to user_path(current_user)
  else
    render text: renderActionInOtherController(UsersController,:show, {:id => 1})
    #@user = User.find(current_user)
    #@microposts = @user.microposts.paginate(page: params[:page])
    #render 'users/show'
  end
end

when I hit post button I get totally nothing in the browser. Moreover, when I try to view tha users/1 page I get the following error(!!):

undefined method `[]' for nil:NilClass

Any help will be really valuable! If you need any other info please inform me!

share|improve this question

2 Answers

up vote 1 down vote accepted

  1. Technically, this terminology is somewhat confusing. By default each method in a controller corresponds to a similarly-named view file, so it may be better to word the question as "Is it good practice to render a view that is not the default view?" And the answer is, of course, it depends. It is a technique commonly used to DRY up controller code and if there are advantages to be gained by it, in your application, I would certainly use it. In fact, the controller code generated by the default resource scaffolding in Rails uses it in the create and update methods. I think you could make the argument that anything in Rails core is, if not a best-practice, at least within the limits of sanity.

  2. With that being said, there may be opportunities for improvement. A common way to handle the same thing would be to route your create and update requests to the same controller action and use the same controller view. If that is not feasible, at least be assured that you will not need to redefine the variables. From the official documentation:

Using render with :action is a frequent source of confusion for Rails newcomers. The specified action is used to determine which view to render, but Rails does not run any of the code for that action in the controller. Any instance variables that you require in the view must be set up in the current action before calling render.

  1. You shouldn't need to do that... Not in this situation, or any other one that I can think of. Any time you see something super hackey like that in Rails is a sign that something probably isn't quite right, and that there are probably better ways to handle the same thing.

Bonus: If you need to go somewhere that already is in charge of setting itself up, and you don't need access to any of the in scope variables, a redirect is probably a better choice. That way, you don't need to re-describe your controller logic in multiple places. Here is an example from your posted code:

    # inefficient and not DRY
    @user = User.find(current_user)
    @microposts = @user.microposts.paginate(page: params[:page])
    render 'users/show'

    # does the same thing as above (in this case)
    redirect_to users_path
share|improve this answer
up vote 0 down vote

I would just like to add that I believe the justification for why the tutorial author implemented a less DRY solution to the Create Microposts action is because if one uses redirect_to instead of render, then they lose the flash errors for the form.

I am not sure if there is a more DRY solution that maintains form flash errors.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.