Ruby on Rails Messaging board

Question

I would definitely love some feedback on this message board app I made. I been doing front end development for the most part and I just want to keep my backend chops up. Let me know anywhere that can be improved.

Here's the github and the heroku

PostController

class PostsController < ApplicationController
 before_action :authenticate_user!
 before_action :set_post, only: [:show, :edit, :update, :destroy]
 # GET /posts
 # GET /posts.json
 def index
 @posts = Post.page(params[:page]).per(5)
 end
 # GET /posts/1
 # GET /posts/1.json
 def show
 end
 # GET /posts/new
 def new
 @post = Post.new
 end
 # GET /posts/1/edit
 def edit
 end
 # POST /posts
 # POST /posts.json
 def create
 @post = Post.new(post_params)
 @post.author_id = current_user.id
 respond_to do |format|
 if @post.save
 format.html { redirect_to @post, notice: 'Post was successfully created.' }
 format.json { render action: 'show', status: :created, location: @post }
 else
 format.html { render action: 'new' }
 format.json { render json: @post.errors, status: :unprocessable_entity }
 end
 end
 end
 # PATCH/PUT /posts/1
 # PATCH/PUT /posts/1.json
 def update
 respond_to do |format|
 if @post.update(post_params)
 format.html { redirect_to @post, notice: 'Post was successfully updated.' }
 format.json { head :no_content }
 else
 format.html { render action: 'edit' }
 format.json { render json: @post.errors, status: :unprocessable_entity }
 end
 end
 end
 # DELETE /posts/1
 # DELETE /posts/1.json
 def destroy
 @post.destroy
 respond_to do |format|
 format.html { redirect_to posts_url }
 format.json { head :no_content }
 end
 end
 private
 # Use callbacks to share common setup or constraints between actions.
 def set_post
 @post = Post.find(params[:id])
 end
 # Never trust parameters from the scary internet, only allow the white list through.
 def post_params
 params.require(:post).permit(:author_id, :body, :title)
 end
end

Post model

class Post < ActiveRecord::Base
 validates :author_id, presence: true
 belongs_to :author,
 class_name: "User",
 foreign_key: :author_id,
 primary_key: :id
 has_many :comments, class_name: "Comment"
 has_many :commenters, through: :comments, source: :user
 default_scope { order("created_at DESC") }
end

CommentsController

class CommentsController < ApplicationController
 before_action :authenticate_user!
 before_filter :load_post
 before_action :set_comment, only: [:show, :edit, :update, :destroy]
 # GET /comments
 # GET /comments.json
 def index
 @comments = @post.comments.all
 end
 # GET /comments/1
 # GET /comments/1.json
 def show
 @comment = @post.comments.find(params[:id])
 end
 # GET /comments/new
 def new
 @comment = @post.comments.new
 end
 # GET /comments/1/edit
 def edit
 @comment = @post.comments.find(params[:id])
 end
 # POST /comments
 # POST /comments.json
 def create
 @comment = @post.comments.new(comment_params)
 @comment.author_id = current_user.id
 @comment.post_id = @post.id
 respond_to do |format|
 if @comment.save
 format.html { redirect_to @post, notice: 'Comment was successfully created.' }
 format.json { render action: 'show', status: :created, location: @post }
 else
 format.html { render action: 'new' }
 format.json { render json: @comment.errors, status: :unprocessable_entity }
 end
 end
 end
 # PATCH/PUT /comments/1
 # PATCH/PUT /comments/1.json
 def update
 @comment = @post.comments.find(params[:id])
 respond_to do |format|
 if @comment.update(comment_params)
 format.html { redirect_to @post, notice: 'Comment was successfully updated.' }
 format.json { head :no_content }
 else
 format.html { render action: 'edit' }
 format.json { render json: @comment.errors, status: :unprocessable_entity }
 end
 end
 end
 # DELETE /comments/1
 # DELETE /comments/1.json
 def destroy
 @comment = @post.comments.find(params[:id])
 @comment.destroy
 respond_to do |format|
 format.html { redirect_to @post }
 format.json { head :no_content }
 end
 end
 private
 # Use callbacks to share common setup or constraints between actions.
 def load_post
 @post = Post.find(params[:post_id])
 end
 def set_comment
 @comment = Comment.find(params[:id])
 end
 # Never trust parameters from the scary internet, only allow the white list through.
 def comment_params
 params.require(:comment).permit(:author_id, :post_id, :body, :title)
 end
end

Comment model

class Comment < ActiveRecord::Base
 validates :post, :body, presence: true
 belongs_to :post,
 class_name: "Post",
 foreign_key: :post_id,
 primary_key: :id
 belongs_to :author,
 class_name: "User",
 foreign_key: :author_id,
 primary_key: :id
 default_scope { order("created_at DESC") }
end

User model

class User < ActiveRecord::Base
 # Include default devise modules. Others available are:
 # :confirmable, :lockable, :timeoutable and :omniauthable
 devise :database_authenticatable, :registerable,
 :recoverable, :rememberable, :trackable, :validatable
 has_many :posts, class_name: "Post", foreign_key: :author_id, primary_key: :id
 has_many :comments
 has_many :commented_posts, through: :comments, source: :post
end

Answer 1

I hope it's the kind of feedback you are looking for, don't take them too personally (there are no big things to say about your code).

for the code itself

• not sure I would have defaulted the scope with "order by" (just a named scope, it's harder to change its mind once in the default_scope)
• don't commit your secret in git (see brakeman tool warnings)
• don't think you need the class_name and primary_key options in the belongs_to :post
• I would use latest stable rails version (5.0.x)
• I don't know if a post is supposed to editable by its author ? Then I would use current_user.posts.find(params[:id]) instead of Post.find(params[:id]) (see the pundit gem for more advanced authorization scheme)
• I would use the shorter hash syntax in erb and be consistent : eg : , method: :delete, :class => 'navbar-link' vs method: :delete, class: 'navbar-link'
• I would not commit DS_store files
• I would add Foreign key constraint in the db (see foreigner or rails 5)
• I'm not a big fan of before action and so on in controllers

• I would add paging to all your index methods (see comments controller)

• When you write code like this @comment = @post.comments.new(comment_params) @comment.author_id = current_user.id @comment.post_id = @post.id I would minimize the @comment access from the controller by merging the author_id in comment_params in one line and call build (not new) on the comments association. for the post_id you are probably missing the "inverse_of:" options

for your toolbox, setup

• rubocop (pick some .rubocop.yml from a project you like the code style)
• brakeman
• bundler audit
• simplecov
• continuous inspection like codeclimate, pullreview, or pronto
• continuous integration tool like travis-ci, codeship,...

purist would tell you to

• use haml or slim instead of erb
• use decent_exposure instead of instance variables to expose model between controllers and views
• don't clutter your views with bootstrap classes
• organize your code with service object or use case classes
• use presenters instead of rails helpers
• use reform to decouple persistent models and form objects

but make your mind on the kind of code you like and pick what you want

• catch22: This is incredible feedback! Thanks @mestachs for all the detail and thought! I am definitely going to go and add these changes. Much appreciated.