Massage your params

Well you’ve launched your fancy new app, and the social aspects are flourishing. In fact, comments are rolling in like it’s christmas. I guess the comments are presents and the site is a small child and your users are Santa?

Anyway, there are a lot of naughty boys and girls out there so now your client, the site operator, requests that comments must be approved prior to publication. The admins will review each comment and then submit the form by using one of three buttons - ‘Save’, ‘Accept’, ‘Reject’. The first is just a normal update, the second two are responsible for changing the publication state of the Comment.

Before the harvest

The Comment model is pretty simple. You should assume that there are query methods (#accepted?, #rejected?, #pending?) defined somewhere in there.

class Comment < ActiveRecord::Base

  STATES = %w{Accepted Rejected Pending}

  validates_inclusion_of :publication_state, :in => STATES

  # pretty sweet code in here, seriously

  def accept
    self.update_attribute :publication_state, 'Accepted'
  end

  def reject
    self.update_attribute :publication_state, 'Rejected'
  end

end

The admin/comments#update action is where the admins POST their comment saves, approvals and rejections to…

class Admin::CommentsController < Admin::BaseController

  def update
    @comment = Comment.find_by_id params[:id]
    if @comment.update_attributes params[:comment]
      if save?
        flash[:success] = 'The comment was edited successfully.'
        redirect_to edit_admin_comment_url(@comment) and return
      elsif accept?
        @comment.accept
        flash[:information] = 'Comment was resaved, accepted and published.'
      elsif reject?
        @comment.reject
        flash[:information] = 'Comment was resaved, but marked as rejected.'
      end
      redirect_to admin_comments_url and return
    else
      flash.now[:failure] = 'The comment could not be saved.'
      render :action => :edit
    end
  end

  protected

  def save?
    params[:commit] == 'Save'
  end

  def accept?
    params[:commit] == 'Accept'
  end

  def reject?
    params[:commit] == 'Reject'
  end

end

In the Comment model, I don’t like those methods that just update the publication_state column. Why do I need to update that and not anything else? I’m pretty sure that there’s only one spot in the entire codebase where those are set.

In the controller, I don’t like how there are two saves in the #update action. Sure, the extra UPDATE is probably minimal impact in regard to performance, but it’s ugly. The only reason those extra saves are there is that the information about the state change is not captured in params[:comment] on the POST request from the form - it’s only in params[:commit] (the form submit button, as it happens).

So what can I do? I want to get down to one save in the #update action, lose some code in the process if I can, and keep my workflow the same for the users.

After the rain

The model changes are simple. I just pull out those two methods and their tests. The new Comment looks like…

class Comment < ActiveRecord::Base

  STATES = %w{Accepted Rejected Pending}

  validates_inclusion_of :publication_state, :in => STATES

  # ...

end

The functional tests don’t need to change - because I want to preserve the same workflow. When a user clicks a button and they know what it’s supposed to do, I want it to keep doing that same thing for them.

But the controller won’t have Comment#accept or Comment#reject available to it anymore, so it’s time to find a new way for the user’s button choice to control the state change.

class Admin::CommentsController < Admin::BaseController

  before_filter :check_state_change, :only => :update

  def update
    @comment = Comment.find_by_id params[:id]
    if @comment.update_attributes params[:comment]
      if save?
        flash[:success] = 'The comment was edited successfully.'
        redirect_to edit_admin_comment_url(@comment) and return
      elsif accept?
        flash[:information] = 'Comment was resaved, accepted and published.'
      elsif reject?
        flash[:information] = 'Comment was resaved, but marked as rejected.'
      end
      redirect_to admin_comments_url and return
    else
      flash.now[:failure] = 'The comment could not be saved.'
      render :action => :edit
    end
  end

  protected

  # ... #save?, #accept? and #reject? stay as they were

  def check_state_change
    if accept? or reject?
      params[:comment][:publication_state] = accept? ? 'Accepted' : 'Rejected'
    end
  end

end

Overall, I like this approach better, even though that new method isn’t that pretty.

I couldn’t populate params[:comment][:publication_state] in my form - because there’s no telling what the right value is before the admin user chooses which submit button to use. Now if they just want to ‘Save’, I’ll leave the state alone - but if they want to ‘Accept’ or ‘Reject’, I’ll massage the params in a before_filter on the way in, and then I only have to save once in the #update action.

It’s not that glamorous, but to me it makes more sense.