This is a companion discussion topic for the original entry at https://thoughtbot.com/upcase/videos/intermediate-part-3
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
To get your app working again just add the following line like so:
include ActiveModel::Model extend ActiveModel::Naming
And everything should work!
In Rails 4, only use
and it should work just fine. Thanks again @JuLoc88
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.
@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
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:
module Following extend ActiveSupport::Concern included do ... end end
class User < ActiveRecord::Base include Following ... end
Worked like a charm!
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?
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.
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?
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.
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…)
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
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:
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
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
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.
Did anyone face the error of ActiveRecord::StatementInvalid in DashboardsController#show for:
@dashboard = Dashboard.new(current_user)
I am using Rails 5 by the way.
It was just naming error on my side. I should not inherit from ApplicationRecord and name it as
TextShout.new instead of