12 Refactoring Notes Controller

Refactoring NotesController

Add a before_action to require users to be logged in before working with notes.

app/controllers/notes_controller.rb

class NotesController < ApplicationController
  before_action :authorize_user

Create another before_action to load a @note instance variable for the appropriate actions...

app/controller/notes_controller.rb

before_action :find_note, only: [:show, :edit, :update, :destroy]

...and write the corresponding private method.

app/controller/notes_controller.rb

def find_note
  @note = Note.find params[:id]
end

We can then remove the @note = Note.find lines from each of those actions.

Now let's DRY up our create and update methods.

The process for setting our flash messages is awfully redundant. Let's extract that process into a private method. The method will need to know whether the action succeeded or failed, so we'll pass that as an argument.

app/controller/notes_controller.rb

def set_flash_for(action_result)
end

If the action succeeded, we typically set flash[:notice], and we set it to t("note.flash.#{action_name}.success") where action_name is create, update, etc. As it happens, action_name is not pseudocode—that actually works!

If the action failed we typically set flash.now[:alert] (as we render, rather than redirect), and we show the failure message from the translation file.

app/controller/notes_controller.rb

def set_flash_for(action_result)
  if action_result
    flash[:notice] = t("note.flash.#{action_name}.success")
  else
    flash.now[:alert] = t("note.flash.#{action_name}.failure")
  end
end

To call this method from our actions, we simply pass it the result of our attempt to save.

app/controller/notes_controller.rb

def create
    @note = Note.new note_params
    set_flash_for @note.save

app/controller/notes_controller.rb

def update
    set_flash_for @note.update(note_params)

We can do the same for destroy too.

app/controller/notes_controller.rb

def destroy
    set_flash_for @note.destroy

Be sure to add the appropriate translations to en.yml

config/locales/en.yml

destroy:
  success: "That note has been deleted."
  failure: "We weren't able to delete that note."

After setting the flash messages in create and update, we either render or redirect. Let's pass that off to a private method too.

app/controller/notes_controller.rb

def render_or_redirect
  if @note.errors.any?
    render :edit
  else
    redirect_to @note
  end
end

It doesn't need an argument, so let's just call it from create and update and be done with it.

app/controller/notes_controller.rb

def create
  @note = Note.new note_params
  set_flash_for @note.save
  render_or_redirect
end

def update
  set_flash_for @note.update(note_params)
  render_or_redirect
end

As for destroy, we'll redirect to new_note_path. That redirect does mean that our flash[:now] message will not display. Feel free to handle that on your own.

app/controller/notes_controller.rb

def destroy
  set_flash_for @note.destroy
  redirect_to new_note_path
end

We don't really need separate show and edit pages, as our users aren't likely to need read-only views of their notes. For the sake of nice URLs, let's render edit in the show view, and get rid of the edit method altogether. We'll even get rid of the route.

app/controller/notes_controller.rb

def show
  render :edit
end

config/routes.rb

resources :notes, except: :edit

All told, our NotesController should look like this:

app/controller/notes_controller.rb

class NotesController < ApplicationController
  before_action :authorize_user
  before_action :find_note, only: [:show, :edit, :update, :destroy]

  def show
    render :edit
  end

  def new
    @note = Note.new
  end

  def create
    @note = Note.new note_params
    set_flash_for @note.save
    render_or_redirect
  end

  def update
    set_flash_for @note.update(note_params)
    render_or_redirect
  end

  def destroy
    set_flash_for @note.destroy
    redirect_to new_note_path
  end

  private

  def note_params
    params.require(:note).permit(:title, :body_html)
  end

  def find_note
    @note = Note.find params[:id]
  end

  def set_flash_for(action_result)
    if action_result
      flash[:notice] = t("note.flash.#{action_name}.success")
    else
      flash.now[:alert] = t("note.flash.#{action_name}.failure")
    end
  end

  def render_or_redirect
    if @note.errors.any?
      render :edit
    else
      redirect_to @note
    end
  end
end

Don't forget to commit your changes.

$ git add .
$ git commit-m "Refactor NotesController."