Part 3

Upcase
1
This is a companion discussion topic for the original entry at https://thoughtbot.com/upcase/videos/intermediate-part-3
2

When using rails 4 you may run into a problem with your app around the 33 min. mark not rendering the timeline even though you have included the line

extend ActiveModel::Naming

To get your app working again just add the following line like so:

include ActiveModel::Model
extend ActiveModel::Naming

And everything should work!

Cheers :slight_smile:

1 Like
3

Thanks @JuLoc88!

4

In Rails 4, only use

include ActiveModel::Model

and it should work just fine. Thanks again @JuLoc88

1 Like
5

Another option

include ActiveModel::Conversion
1 Like
6

Hello everyone,

I get an error when I click the follow button and I can’t figure out how to fix it.

NoMethodError in FollowingRelationshipsController#create

undefined method `name' for nil:NilClass

app/controllers/following_relationships_controller.rb:4:in `create'
```

```ruby
# following_relationships_controller.rb
def create
  user = User.find(params[:user_id])
  current_user.followed_users << user # This is line 4
  redirect_to user 
end
```

I didn't set any methods named name. The only place I found a reference to something called name was in the schema when the index where created for the following_relationships table.

```ruby
add_index "following_relationships", ["followed_user_id"], :name => "index_following_relationships_on_followed_user_id"
add_index "following_relationships", ["follower_id"], :name => "index_following_relationships_on_follower_id"
```

How should I approach this issue?

Thank you for your help.
7

@aclapinpepin Do you have the full stack trace for this? If you follow the rest of the lesson he refactors out the user creation into a private method called #user. Also take a look at his his full source at GitHub - halogenandtoast/intermediate_rails: intermediate_rails

8

Around the 45 minute mark, he introduces Concerns. The usage for these is a little different in rails 4, so here is the code you need to make sure the Following concern is included in your User model:

app/models/concerns/following.rb

module Following
  extend ActiveSupport::Concern

  included do
  ...
  end
end

app/models/user.rb

class User < ActiveRecord::Base
  include Following
  ...
end
9

Worked like a charm!

10

I have a question. The private :user method in the screencast uses memoization so that it won’t hit the database multiple times. But what happens the subsequent request goes to create or destroy action but with a different user id? Will it look up in the database or use the cached user? If it uses the cashed user, wouldn’t that introduce some weird behavior?

11

My understanding is that the subsequent request will create a new instance of the controller, so it should be ok. The memoization in the private user method is to avoid repeating the same query twice within the create or destroy methods.:slight_smile:

12

I’ve tried to tackle the assignment given in the video to DRY up the create action in the text/photo shout controllers and would love some feedback.

I wound up moving the create action from both of the controllers into a shared app/controllers/concerns/create_shout.rb.

This dries up the code a bit, but I feel like it makes reading my controller less straightforward. Can anyone share a better solution?

Thanks

13

Hi Josh,

What I was thinking about for refactoring was taking out the build_content method, since it is virtually the same in text shouts and photo shouts.

I was also considering taking out the if else statement from the create action and turning that into a method or 2 but leaving the redirect_to dashboard_path in since it happens in both branches of the conditional. I have trouble taking out conditionals though and it often takes a long time trying to channel my inner Sandi Metz to get it right lol.

If I get around to the refactoring, I will link to the code.

Cheers,
Nathan

1 Like
14

Hi Josh,

I did the same thing - put the create method in a Concern module. That certainly DRYs up the controller, but yeah, personally I think it reduces readability.

I also simplified the logic a bit - no need to declare the content variable at the top of the create action, and no need for an if else, since the only thing in the if is also in the else (i.e. redirecting).

(I also changed .save to a new .save_if_valid method, which I defined in the Shout class to check whether the content is valid - and added a validation on text_shouts so you can’t shout an empty string. This isn’t really refactoring but I noticed the validation we’d created for Shouts no longer exists with the polymorphic approach…)

15

Hi All,

Working on the refactoring for the shouts recommended at the end of part 3 for intermediate-ruby. I’m a little lost as to how to extract the build_content method into a module with the differing params for photos and text. Can someone clarify how to work through this or give me a good reference for concerns? Looked around but couldn’t find anything helpful

16

Hi Megan! I played around with this tonight and this is what I came up with. Problem is, I think it makes the code less readable…but thought I’d share and see what people have too say. Please, anyone that reads this, provide feedback if this is poor practice. I’m still new at Rails / coding in general.

First thing I did was say “Both the photo shout and the text shouts are both shouts. Which is probably why so much of the logic is duplicated here. Why not have them inherit from the ShoutsController instead of from ApplicationController and move as much of the duplicate logic into ShoutsController as possible?”

class PhotoShoutsController < ShoutsController 
end 

class TextShoutsController < ShoutsController 
end

Then I removed #create out of the PhotoShoutsController and the TextShoutsController and put it in my ShoutsController.

class ShoutsController < ApplicationController
  def create
    shout = current_user.shouts.build(content: build_content)

    if shout.save
      redirect_to dashboard_path
    else
      redirect_to dashboard_path, notice: "Could not shout"
    end
  end

  def show
    @shout = Shout.find(params[:id])
  end
end

Then, much like you, I was troubled by the duplicate code for both the #build_content and for the method to gather the parameter data (#photo_shout_parameters and #text_shout_parameters). So I pulled both of them out and added them to the ShoutsController with a general naming convention I could envision using.

  def build_content
    ClassName.new(shout_parameters)
  end
  
  def shout_parameters
    params.require(:shout_type_symbol).permit(:shout_type_column)
  end

Next I had to figure out how to tell those methods what symbols (:text_shout or :photo_shout), class names (TextShout or PhotoShout), and column types (:image or :body) to use. So I just created methods for each of them and returned the proper values based on the type of controller in the params.

So if a text shout is being created, params[:controller] would return the string “text_shouts”. So I used the params[:controller] to create the type of class, symbol, and columns I needed like so:

  def controller_type_singularized
    params[:controller].chomp("s")
  end

  def class_name
    controller_type_singularized.classify.constantize
  end

  def model_symbol
    controller_type_singularized.to_sym
  end

  def column_type
    return :body if controller_type_singularized == "text_shout"
    :image
  end

  def build_content
    class_name.new(shout_parameters)
  end

  def shout_parameters
    params.require(model_symbol).permit(column_type)
  end

First the #controller_type_singularized will return “text_shout” or “photo_shout”. The #class_name uses #classify and #constantize to turn a string into a class (“text_shout” into TextShout). I used #model_symbol to turn “text_shout” into :text_shout. And used #column_type to return either :body or :image based again on the controller.

So here is how everything ended up:

##ShoutsController

class ShoutsController < ApplicationController
  def create
    shout = current_user.shouts.build(content: build_content)

    if shout.save
      redirect_to dashboard_path
    else
      redirect_to dashboard_path, notice: "Could not shout"
    end
  end

  def show
    @shout = Shout.find(params[:id])
  end

  private

  def controller_type_singularized
    params[:controller].chomp("s")
  end

  def class_name
    controller_type_singularized.classify.constantize
  end

  def model_symbol
    controller_type_singularized.to_sym
  end

  def column_type
    return :body if controller_type_singularized == "text_shout"
    :image
  end

  def build_content
    class_name.new(shout_parameters)
  end

  def shout_parameters
    params.require(model_symbol).permit(column_type)
  end
end

##PhotoShoutsController

class PhotoShoutsController < ShoutsController
  before_action :require_image, only: [:create]

  private

  def require_image
    if params[:photo_shout].blank?
      redirect_to dashboard_path, notice: "You must provide a photo"
    end
  end
end

##TextShoutsController

class TextShoutsController < ShoutsController
  before_action :require_text, only: [:create]

  private

  def require_text
    if params[:text_shout]["body"].blank?
      redirect_to dashboard_path, notice: "You must provide text"
    end
  end
end

And thats it…

But as I said, my instinct tells me that this made it harder to read. Also, I don’t like the before_action duplication in the TextShoutsController and the PhotoShoutsController. I’d added those because a new shout was being created even if the user didn’t enter any text or a photo.

Feedback anyone?

17

Did anyone face the error of ActiveRecord::StatementInvalid in DashboardsController#show for:

@dashboard = Dashboard.new(current_user)
18

I am using Rails 5 by the way.

19

It was just naming error on my side. I should not inherit from ApplicationRecord and name it as TextShout.new instead of Textshout.new