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