Mark Daggett's Blog

Innovator & Bricoleur

Introducing ActsAsModerated

Periodically sites that incorporate user generated content need a way to moderate the incoming stream before publishing it to the site. Typically this is accomplished by putting the content in a queue and allowing moderators to explicitly accept or reject content. I needed such functionality for a site I was working on so I wrote the “ActsAsModerated” plugin which allows specific columns of a model to be audited by a moderator at some later point.

ActsAsModerated is good for:

  • spot-checking user generated content
  • being notified when new content is created
  • tracking changes within a record
  • spam checking

Moreover, this plugin allows for custom callbacks and validations around the moderation event. This flexibility gives developers the ability to augment the moderation flow by stacking custom rules as needed. For example, a developer write code to assign a reputation score to a user and increment that score for every non-spam contribution they make. This would mean that you could auto approve any content that is made by a user above a certain score, thereby reducing the workload on moderators. I have been using this plugin for over a year on a very high-volume site and it has held up quite well, and so I thought I’d share it with the rest of the Ruby on Rails community.

Download & Install

For those with ADD you can find acts_as_moderated here: https://github.com/heavysixer/acts_as_moderated

Setup

Create the moderated_records table like so:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
create_table "moderation_records", :force => true do |t|
  t.integer  "recordable_id"
  t.string   "recordable_type"
  t.integer  "state_id",             :default => 0
  t.integer  "decision_id",          :default => 0
  t.boolean  "flagged",              :default => false
  t.integer  "moderator_id"
  t.string   "reason"
  t.datetime "created_at"
  t.datetime "updated_at"
  t.text     "inspected_attributes"
  t.boolean  "rejected",             :default => false
end

add_index "moderation_records", ["decision_id"], :name => "index_moderation_records_on_decision_id"
add_index "moderation_records", ["flagged"], :name => "index_moderation_records_on_flagged"
add_index "moderation_records", ["moderator_id"], :name => "index_moderation_records_on_moderator_id"
add_index "moderation_records", ["recordable_id", "recordable_type"], :name => "index_moderation_records_on_recordable_id_and_recordable_type"
add_index "moderation_records", ["rejected"], :name => "index_moderation_records_on_rejected"
add_index "moderation_records", ["state_id"], :name => "index_moderation_records_on_state_id"

Usage

ActsAsModerated has two integration points. The first is within the model(s) to be moderated, which can be done like so:

1
2
3
class Comment < ActiveRecord::Base
  acts_as_moderated :body
end

The second integration point is the class that acts as the moderator. Typically this is some user or account class. The idea is that behind this integration point is to create an audit trail for decisions made by the moderator if you ever need to watch the watcher.

1
2
3
  class Account < ActiveRecord::Base
    acts_as_moderator
  end

The plugin also supports an after_moderation callback on the record being moderated, which you can use to take action based on what the moderator did. For example:

  • Delete the record if it is inappropriate or spam
  • Email the content creator that their content has been approved / denied

There are several dynamically created methods added to every acts_as_moderated class which moderators can use as a shortcut for making decisions. For example where @moderator is an object with acts_as_moderator applied:

1
2
  @comment.marked_spam_by_moderator(@moderator)
  @comment.marked_scam_by_moderator(@moderator, :reason => 'this is obviously a scam, please delete.')

Default Ordering, Callbacks & Flagging

The ModerationRecord class has a named_scope called queue, which will return records sorted oldest to newest. However, if you flag a record it will be returned first regardless of its age relative to unflagged records. This is useful if you want to ensure that moderators see potentially dangerous records first. A good way to flag a record is using the after_moderated callback for example:

1
2
3
4
5
6
7
class Comment < ActiveRecord::Base
  acts_as_moderated :body

  def after_moderated(moderation_record)
    moderation_record.flag! if body =~ /viagra/i
  end
end

The moderation record will attempt to make callbacks on the model being moderated after a record is first created after_moderation and when a moderator rejects a record after_rejection here is an example of what they might do:

1
2
3
4
5
6
7
8
9
10
11
class Story < ActiveRecord::Base
  acts_as_moderated :body

  def after_moderated(moderation_record)
    update_attribute(:moderated, true)
  end

  def after_rejection(moderation_record)
    update_attribute(:rejected, true)
  end
end

Skipping moderation

The moderation plugin adds an attr_accessor called skip_moderation, which when set to true will prevent a moderation record from being created for that instance of a save. This is useful if you need to create records programmatically, which don’t need to be moderated initially but will need to be moderated at some later point. For example:

1
2
  @story = ModeratedStory.create #creates a moderation record
  @story = ModeratedStory.create(:skip_moderation => true) #does not create a moderation record

A word to the wise however, since this attr_accessor prevents records from being moderated you will want to protect it from mass_assignment in your model.

Known Bugs

Presently if you use the :always_moderate flag on a STI model it will produce a never-ending series of record updates. I’ll keep working on this bug, in the meantime please do investigate!

2011 Heaven - Awesome Sauce From the Year That Was

Soon the media waves will be jammed with “Best Of” lists as the last days of 2011 are ripped off the calendar. To get ahead of the curve i’ve decided to create a list of my favorite things from 2011, in no particular order.

Most Helpful Coders

I have a tendency to be a help vampire occasionally, it is a urge i try and suppress where possible. However, I am addicted to smart people and that often leads me down the path of ruination. The people on this list are more helpful and patient than I thought humanly possible. They were always available to answer my often hair-brained and/or panic-stricken questions. This list is not ranked in any particular order:

  • Wayne E. Seguin - For bailing me out on RVM / SM nonsense
  • Michal Papis - For bailing me out on RVM / SM nonsense
  • Vince Toms - For bailing me out on iPhone, Unix, freeBSD and just about any other technical task you can think of

Best Purchases of 2011

  • Breville Barista Express BES860XL machine with grinder I cannot recommend this machine highly enough. I was a barista all through college and have always had a distain for at home expresso machines. The real beauty of this machine is that it gets enough pressure to pull a proper shot, and has a professional grade steam wand, and grinder that can be fine-tuned to boot.
  • Where Good Ideas Come From I am a Steven Johnson fan-boy and his latest offering does not disappoint. It is an enthralling trip through the environments, histories and frames of mind that spark the idea of the next big thing.

Best Audio

Best Time Sink

CSS Anti-patterns

Over the last month of two I have been working on a large enterprise Rails site. The backend has been implemented by a core group of developers but the frontend CSS and HTML have been handed off several times. As I worked to refactor some of the views I noticed several anti-patterns reoccurring in the code that I thought I would highlight and propose work arounds. While all of the examples I present in this file are extracted directly from the site in question I have seen them occur over and over again on other projects.

1. Unclear Naming Conventions

If a tag in an HTML page performs a task then a good CSS name should describe what the job is. It should have semantic value and should not where possible describe only visual attributes.

Anti-pattern Example
1
2
3
4
.teal {
color: #1EC5E9;
font-style: italic
}

This class name gives very little semantic value to tag it is applied to. The class “teal” only describes the visualness of the tag. Furthermore this name is only relevant while the visual design of the page stays the same. If the design changes then every instance of the “teal” class will need to be removed throughout the codebase if the color changes. What is worse is if the CSS attributes within the class are updated without changing the name then the name becomes meaningless. A better approach would be to understand what the teal color is for. If it is meant to draw emphasis to a bit of text then choose a name like “highlight” so that it is more clear what job it is to perform.

2. Invalid Declarations

When writing CSS periodically run your classes through a CSS validator to ensure its syntax is correct. The problem with invalid declarations is that they may not break the tag in question but may corrupt the rest of the cascade causing classes defined later in the file from working. Whenever I am hunting down a CSS error the first thing I do is ensure I am working off a valid document. I can’t tell you how many times just fixing the code in one part of the file fixed mystery bugs that appeared lower in the document.

3. Unnecessary verboseness

Where possible try to compact your CSS attributes into one line:

Anti-pattern Example
1
2
3
4
  background: #2175BE;
  background-image: url(../images/bottomshadownav.png);
  background-position: bottom;
  background-repeat: repeat-x;
Improved Version
1
  background: #2175BE url(../images/bottomshadownav.png) repeat-x bottom;

4. Overly Specific Classes

I strive to make CSS classes do only one thing. That does not mean that my classes only have only one attribute declaration, but it means that I try to assign them only one job to perform. This allows me to build up complex behaviors and visuals to html tags by combining several simple CSS classes. Resist the urge to add properties to your class that doesn’t describe the job it does. This will also make your HTML more clear to read, and easier to extend. Consider the two examples below:

Anti-pattern Example
1
2
3
4
  .slider {
    height: 27px;
    float: left;
  }
Improved Version
1
2
3
4
5
6
  .slider {
    height: 27px;
  }
  .left {
    float: left;
  }

5. Browser Specific Overrides

Where possible resist the urge to place browser specific hacks in the master CSS file. The proper approach is to peel these classes off into an override stylesheet that will be loaded only when the user is using that particular browser. The reason we put override classes in their own file is for two reasons. It simplifies the master CSS file, and gives the developer an easy way to eliminate CSS from the site when the site no longer needs to support a deprecated browser.

Anti-pattern Example
1
2
3
  .decile img {
  _margin: 0px /* IE 6 Hack */
  }

6. Needless Duplication

Part of writing good CSS is to generalize common tasks where possible. In the example below you’ll notice that both classes are essentially the same.

Anti-pattern Example
1
2
3
4
5
6
7
8
  .comparator a {
  margin-right: 3px;
  padding: 3px 13px 3px 13px;
  }
  .ambition a {
  padding: 3px 13px 3px 13px;
  margin-right: 3px
  }
Improved Version
1
2
3
4
  a.link {
    padding: 3px 10px 3px 10px;
    margin-right: 3px
  }

7. Overuse of exclamation point

The “!” override is a brute force method to ensure an attribute of your class cannot be overwritten. This should be used as a last resort and not to mask poor design or abstraction of classes. Overuse of the exclamation point is like a shouting match between all your CSS classes, almost never ends well.

8. “Cargo Culting” through Copy + Paste

Copying CSS en masse and pasting it into the document without making fundamental changes to the class can be a sign of disconnect between the designer and developer or a signal that the developer / designer doesn’t know (or care) about how the copied CSS works. Bulk copying CSS creates deadweight in the CSS file and makes it harder to maintain and extend.

In the first case where a developer is implementing a design given to them by a front-end developer it is entirely possible the developer won’t know how the CSS works. Later, they may find themselves needing to replicate a style elsewhere in the site. Often this means they bulk copy several classes and paste them elsewhere in the document. Typically, they make a trivial changes needed e.g. change the font-size and then rename the class. The proper approach would be to abstract the first class so that it is more generalized and can service both instances of how it needs to be implemented.

However, I found several examples where entire blocks of code appeared three and four times in the same file with absolutely no difference! This is absolutely unacceptable in a professional product.

9. Unused Classes

With any site under constant development it is easy for classes to become unused within the html. Developers and designers must make it part of their development process to remove unused classes from the CSS otherwise the site can become bogged down with all the deadweight inside the CSS file. There are projects like “deadweight” which are specifically dedicated to helping you prune your CSS of unused code.

10. Needless Namespacing

Namespacing is used to change the way a child class works when its parent class changes. This is a great way to keep your HTML and CSS flexible because it means that a single change to the parent element can change the look and feel of all the containing children. However, if you over namespace your CSS by binding it to specific html tags where not absolutely needed you make the CSS brittle and hard to maintain.

Anti-pattern Example
1
2
3
4
5
  .qipp-inputs table tr.metric td.metric_name {
  border-bottom: 1px dotted #B6CBD2;
  padding-left: 20px;
  text-align: left !important;
  }
Improved Example
1
2
3
4
5
  .qipp-inputs .metric td.metric_name {
  border-bottom: 1px dotted #B6CBD2;
  padding-left: 20px;
  text-align: left !important;
  }

I welcome any discussion or feedback on these patterns and would love to improve this document where possible. What anti-patterns have you seen in CSS files you’ve worked on?

Transactions in Rails

Recently I was tasked to write tests for the transactions of an existing application. This gave me the opportunity to learn more about the codebase, while also improving the test coverage. Generally, most of the transaction code looked fine. However, there were some instances where transactions were used incorrectly or inefficiently. I assumed this is just because of a misunderstanding of how transactions work within Rails, and so I thought iʼd take some time and give an overview of the common errors I found and some best practices for using transactions in Rails.

Let me also state at the beginning that most of these examples are not my own, they come directly from the Rails source, which give example usages of applying transactions RTFM FTW.

http://api.rubyonrails.org/classes/ActiveRecord/Transactions/ClassMethods.html

Reasons for transactions

We use transactions as a protective wrapper around SQL statements to ensure changes to the database only occur when all actions succeed together. Transaction help the developer enforce data integrity within the application. The canonical example of transactions is the banking method where funds are withdrawn from one account and deposited into the next. If either of these steps fail, then the entire process should be reset. This example can be described in code this way:

1
2
3
4
ActiveRecord::Base.transaction do
  david.withdrawal(100)
  mary.deposit(100)
end

In Rails transactions are available as class and instance methods for all ActiveRecord models. This means either of these approaches are equally valid:

1
2
3
4
5
6
7
8
9
10
11
Client.transaction do
  @client.users.create!
  @user.clients(true).first.destroy!
  Product.first.destroy!
end

@client.transaction do
  @client.users.create!
  @user.clients(true).first.destroy!
  Product.first.destroy!
end

You may also notice that there are several different model classes referenced in these transactions. It is perfectly fine to mix model types inside a single transaction block. This is because the transaction is bound to the database connection not the model instance. As a rule, transactions are only needed when changes to multiple records must succeed as a single unit. Additionally, Rails already wraps the #save and #destroy methods in a transaction, therefore a transaction is never needed when updating a single record.

Transaction Rollback Triggers

Transactions reset the state of records through a process called a rollback. In Rails, rollbacks are only triggered by an exception. This is a crucial point to understand; I saw several transaction blocks that would never rollback because the containing code could not throw an exception. Here I have slightly modified our banking example from before:

1
2
3
4
ActiveRecord::Base.transaction do
  david.update_attribute(:amount, david.amount -100)
  mary.update_attribute(:amount, 100)
end

In Rails #update_attribute is designed not to throw an exception when an update fails. It returns false, and for this reason you should ensure that the methods used throw an exception upon failure. A better way to write the previous example would be:

1
2
3
4
ActiveRecord::Base.transaction do
  david.update_attributes!(:amount => -100)
  mary.update_attributes!(:amount => 100)
end

Note: The bang modifier (!) is a rails convention for a method which will throw an exception upon failure.

I also saw examples in the code where records were found inside of transactions using magic finders #find_by. By design magic finders will return nil when no record is returned. This is in contrast to the normal #find_ method which throws an ActiveRecord::RecordNotFound exception. Consider this example:

1
2
3
4
5
6
7
ActiveRecord::Base.transaction do
  david = User.find_by_name("david")
  if(david.id != john.id)
    john.update_attributes!(:amount => -100)
    mary.update_attributes!(:amount => 100)
  end
end

Do you see the logic error? The nil object will have an id attribute and it will mask the fact that the desired record was not returned. While this doesn’t cause an error in the transaction it will still lead to a loss of data integrity, because the application is updating where it shouldn’t. Remember a transaction defines a block of code which must succeed as an atomic unit. This means we should raise an exception when a logical dependency like this is not met. Here is how the code should be written:

1
2
3
4
5
6
7
8
ActiveRecord::Base.transaction do
  david = User.find_by_name("david")
  raise ActiveRecord::RecordNotFound if david.nil?
  if(david.id != john.id)
    john.update_attributes!(:amount => -100)
    mary.update_attributes!(:amount => 100)
  end
end

When the exception occurs it will be raised outside of the transaction block after the rollback has completed. Your application code must be ready to catch this exception as it bubbles up through the application stack.

It is also possible to invalidate a transaction without raising an exception that will propagate upwards by using ActiveRecord::Rollback. This special exception allows you to invalidate a transaction and reset the database records without needing to rescue elsewhere in your code.

When To Use Nested Transactions

One of the common mistakes I saw in the codebase is the misuse or overuse of nested transactions. When you nest a transaction inside another transaction this has the effect of making the child transaction a part of the parent. This can have surprising results, take this example from the Rails API documentation:

1
2
3
4
5
6
7
User.transaction do
  User.create(:username => 'Kotori')
  User.transaction do
    User.create(:username => 'Nemu')
    raise ActiveRecord::Rollback
  end
end

As mentioned previously ActiveRecord::Rollback does not propagate outside of the containing transaction block and so the parent transaction does not receive the exception nested inside the child. Since the contents of the child transaction are lumped into the parent transaction both records are created! I find it easier to think of nested transactions like the child who dumps its contents into the parent container, leaving the child transaction empty.

To ensure a rollback is received by the parent transaction you must add the :requires_new => true. option to the child transaction. Again using the example from Rails source you would trigger a parent’s Rollback like this:

1
2
3
4
5
6
7
User.transaction do
  User.create(:username => 'Kotori')
  User.transaction(:requires_new => true) do
    User.create(:username => 'Nemu')
    raise ActiveRecord::Rollback
  end
end

A transaction only acts upon the current database connection. If your application writes to multiple databases at once you will need to wrap the method inside a nested transaction. For example:

1
2
3
4
5
6
Client.transaction do
  Product.transaction do
    product.buy(@quantity)
    client.update_attributes!(:sales_count => @sales_count + 1)
  end
end

Callback around Transactions

As mentioned previously #save and #destroy are wrapped inside a transaction. This means that callbacks like #after_save is still part of the active transaction which might still be rolled back! Therefore if you want code to execute that is guaranteed to execute outside of the transaction use one of the transaction specific callbacks like #after_commit or #after_rollback.

Transaction Gotchas

Do not catch an ActiveRecord::RecordInvalid exception inside a transaction because the this exception invalidates the transaction on some databases like Postgres. Once the transaction has been invalidated you must restart it from the beginning for it to work correctly.

When testing rollbacks or transaction related callbacks that happen after rollback you will want to turn off transactional_fixtures which are on by default in most test frameworks.

Common Anti-patterns To Avoid

  1. Using a transaction when only a single record is updated
  2. Needlessly nesting transactions
  3. Transactions that contain code, which won’t cause a rollback
  4. Use of transactions in a controller