Let it REST

You are building a boring blog application. A blog has many comments. Your client gives you this user story:

So that I can report inappropriate comments, as a user, I can mark a comment as spam.

So you add a spam boolean attribute to the Comment model, and use the same PUT to comments to handle the update:

class CommentsController < ApplicationController
  def update
    @comment = Comment.find(params[:id])
    if @comment.update_attributes(params[:comments])
      flash[:notice] = "Comment updated successfully"
      # redirect_to ...
    else
      flash[:error] = "Something broke"
      # redirect_to ...
    end
  end
end

By sending the right params down from the view, this can still work. But a day passes by and people start abusing the darn spam button. This is the internet after all. So your client adds a couple of stories:

So that I’m aware I screwed up, as a user, I receive an email notification when my comments get spammed. So that I can moderate spammed comments, as an admin, I receive an email notification when comments are spammed.

There’s now more going on when we spam a comment. A first shot at implementing these stories could make use of ActiveRecord’s dirty attributes to figure out if the spam boolean changed from false to true, in which case we deliver the two notifications:

class Comment < ActiveRecord::Base
  before_save :send_spam_notifications, :if => marking_as_spam?
  def send_spam_notifications
    Mailer.comment_owner_spam_notification(self).deliver
    Mailer.admin_spam_notification(self).deliver
  end

  def marking_as_spam?
    self.spam_changed? && self.spam?
  end
end

Nice and clean, right? Wrong.I think there’s a couple of problems with this. First of all, we are cluttering our comment class with conditional callbacks. Sometimes you have no choice, but I’d rather avoid them when possible. Secondly, we’re using the same HTTP endpoint and controller action for two very different things. One is that of editing my comment, and the other is for marking a comment as spam — which is arguably an SRP violation (PDF link).

We have a couple of options for cleaning up. We could create a #update_spam_status action on the comments controller, but this deviates from our otherwise RESTful API. Instead, let’s break it out to it’s own resource called Spams, and instead of a PUT to comment, let’s POST to spam:

class SpamsController < ApplicationController
  def create
    @comment = Comment.find(params[:id])
    if @comment.mark_as_spam
      flash[:notice] = "Comment marked as spam"
      # redirect_to ...
    else
      # ...
    end
  end
end

class Comment < ActiveRecord::Base
  def mark_as_spam
    self.spam = true
    send_spam_notifications if valid?
    save
  end
end

Our view can make use of a button_to that POSTs to the spams resource, instead of relying on updating via the comments resource. Note how a RESTful resource does not need to match 1-to-1 with a model, and this creates a much cleaner interface overall — with a few advantages:

• You clean up your model by creating a #mark_as_spam method that is clear and to the point, untangling some of the callback soup that can start to build up on models.
• You are creating a level of abstraction that allows you to code to an interface, making it trivial to change #mark_as_spam‘s implementation to anything else, like an Akismet call, or a more complex Spam class.
• Splitting it out allows us to properly scope the CommentsController#update’s finder to the current user (@comment = current_user.comments.find(params[:id])), allowing any user to mark comments as spam, and only commenters to update their own comments.
• You can easily specify a more meaningful flash message for each case.

The question then arises, when should we split out to a new resource? As always, it depends. For the most part it’s whenever it will help clean up your models and controllers. I find that if the update triggers any logic at all, it’s worth breaking out to its own resource and corresponding model method that encapsulates that logic.