Showing posts with label programming. Show all posts
Showing posts with label programming. Show all posts

Saturday, May 4, 2013

Why did Heroku stop precompiling my assets?

I come into work today to this error:

ActionView::Template::Error ('twitter/bootstrap/bootstrap.less' wasn't found.

Odd...It works in development. What could have happened?

The search

I ran the application locally with the development environment and got the same error. Great, at least it's reproducible. Let's hit the Googles...

Hmm, some mentions in twitter-bootstrap-rails and some scattered StackOverflow questions, but nothing that works. Then I noticed Heroku was no longer precompiling the assets on deploy.

The discovery

The Heroku docs have this glorious quote:

The most common cause of failures in assets:precompile is an app that relies on having its environment present to boot. Your app's config vars are not present in the environment during slug compilation, so you should take steps to handle the nil case for config vars (and add-on resources) in your initializers.

A recent change added some configuration variables in application.rb that referenced some environment variables.

The fix

Based on some more documentation and community help, I moved the configuration into an initializer that is loaded after the slug compilation when it can access the environment variables.

Deploy. Working. #winning.

Wednesday, April 17, 2013

Using ActiveSupport::TimeWithZone without Rails

My fast spec test suite does not load Rails (otherwise it's not so fast, right?), but I want to use the niceness of ActiveSupport::TimeWithZone to create a DateRange class like so:

class DateRange
  class DateRangeError < StandardError; end

  def initialize(range)
    @range = range
    raise_if_not_utc
  end

  def start_date
    range.first.beginning_of_day
  end

  def end_date
    range.last.end_of_day
  end

  private

  attr_reader :range

  def raise_if_not_utc
    if start_date.zone != 'UTC' || end_date.zone != 'UTC'
      raise DateRangeError.new('The date ranges must be in UTC.')
    end
  end
end

The corresponding tests didn't like Time.zone - they thought it was nil. Which it is.

The Fix

The tests need the proper ActiveSupport module loaded, and the time zone needs to be set.

require 'active_support/core_ext'
Time.zone = 'UTC'

Now the time zone is set and the tests will work as expected.

let(:range) { daterange.new(10.days.ago..time.zone.now) }

describe '#start_date' do
  it 'gets the beginning of the day for the earliest date in the range' do
    expect(range.start_date.to_i).to eq(Time.zone.parse('1999-12-31 00:00:00').to_i)
  end

  it 'is in UTC' do
    expect(range.start_date.zone).to eq('UTC')
  end
end

Thursday, April 11, 2013

RSpec won't clear the database between test runs

A quick one that can save a major headache. It seems that RSpec won't clear the database of a record that is defined outside of a let statement, a begin block, or an it block.

describe '#some_method' do
  record = FactoryGirl.create(:my_record)

  it 'does something' do
    # ...
  end
end

In this case, record will not get cleared out between test runs. It's noticeable when checking that a query only returns certain records because it could include an extra one that's unexpected.

The solution is to either declare the record in a let statement, a begin block, or an it block.

Headache gone.

Saturday, July 7, 2012

Eventual Consistency In UI Design

A recent article from Los Techies resonated with something I also did recently at work, and it's nice to see others come up with similar ideas independently. Hopefully that means they're good ideas.

"Eventual consistency in interaction design" presents the idea of giving the user immediate feedback for an action that is not immediate. It combines a synchronous call (a message to a user after he interacts with a page) with an asynchronous one (processing the results of that action in a background task). The benefits are obvious: the user gets immediate feedback that something happened, and he knows that, while it may not have happened right away, he's not stuck in spinner hell, waiting for some sort of response.

When a user does certain actions on our site, we create an activity to put it in a feed to either display or to send it off to a third party. We create the activity immediately and put it in a job queue to do whatever functionality it requires later. The job queue is fast, but it can take a minute or so before it processes the new activity.

That's a minute the user shouldn't need to wait for.

So when we schedule the job, we also save the related data, the data we're sending off to a third party site, so the user sees the results immediately. To him, his action happened in real time and he never notices the difference.

One obvious question is what if the user tries to do something on our site with the data that is supposedly linked to another site because he assumes they've been synchronized? Since everything that goes to the other site is done asynchronously, his other actions just get added to the job queue after the first action. If he goes to the other site, the job is generally fast enough that it will have processed the original activity (again, since most jobs run in under a minute). If it becomes a problem where users are getting to the other site too quickly, we'll be able to come up with language more akin to "we're processing your request." But this current implementation provides a nice solution for now.

Like the article mentions, "If you have users that have to wait to have the view model updated to see their results, you have built the wrong user interface." In our case, the user does not have to wait for an update, even if that update hasn't happened yet.

Wednesday, June 20, 2012

Squashing bugs with Git

We found a bug at work where a css style changed, but it wasn't immediately obvious where it had been introduced. To fix this bug, I went back through the git history and checked out one in the recent past to see if the bug had been introduced there. This only took a few tries, and then I worked back toward the present to find when the styling had been modified.

I found the offending commit by checking out the sass file of that particular commit:

git checkout {commit_number} path/to/stylesheet.sass

Now I've got the most recent code plus the file that fixes the problem. To see what had been changed, I did a diff on the two files: the older one without the bug and the next one with the bug:

git diff {older_commit} {newer_commit} path/to/stylesheet.sass

There are a lot of changes, so I looked for anything that would be obvious (margins in this case). After that, it was modifying the styling to fix the bug and done.

Git plays nice.

Sunday, June 10, 2012

Rails controller tests without Rails

In our continued path to faster tests, we've tried doing controller tests without loading Rails and using only what's minimally required to fully test the functionality. the idea is that not only will the tests be faster, but they will drive the controller's design to be thinner.

How did it work out? First the background.

We implemented a controller that received an OAuth callback from another site to allow their users to directly login to Crowdcast. The required code was minimal, so we tried to test minimally as well.

Since we're not loading Rails, we need to manually load some required libraries for the controller. We need enough to load the controller and its dependencies (in this case, a url generator class and the controller itself) successfully.

require 'uri'
require 'active_support'
require 'fast_spec/spec_helper'
require 'lib/crowdcast/url_generator'
require 'app/controllers/oauth_controller'

We also need to stub out calls to models, both ActiveRecord and plain old Ruby objects the controller uses. Here is where we realize how many of these we need and use the pain of stubbing yet another class to drive the design to remove that dependency.

stub_class 'UserSession'
stub_active_record_model 'Site'
stub_active_record_model 'SalesforceConnection'

Some are necessary, such as those for Authlogic and the relevant AR classes we're calling, but we were able to move calls to others out of the controller to make the tests easier to write and the code easier to read and maintain. All the usual good stuff we get from TDD and OOP.

Here's an example:

describe OauthController do
  let(:controller) { OauthController.new }

  describe "#callback" do
    let(:user_attributes) {{ :email => "email" }}
    let(:user) { stub("user", :single_access_token => "foo") }
    let(:site) { stub("site") }
    let(:redirect_to_url) { "https://foo.example.com:8080?foo=bar&baz=quux" }
    let(:state) { CGI.escape({:url => redirect_to_url, :subdomain => "foo" }.to_json) }

  before do
    controller.stub(:params) {{ :code => "foo", :state => state }}
    controller.stub(:current_user) { user }
    controller.stub(:session) {{ :return_to => "/return_path" }}
    Crowdcast::Config.stub(:salesforce) {{ :id => "bar" }}

    Salesforce::UserResolver.stub(:user_attributes_by_auth_code).with({ :id => "bar" }, "foo").and_return(user_attributes)
    SalesforceConnection.stub(:connect).and_return(user)
    Site.stub(:find_by_subdomain).with("foo").and_return(site)
  end

  it "creates a SalesforceConnection connection" do
    SalesforceConnection.should_receive(:connect).once.with(user, user_attributes)
    controller.callback
  end

  it "redirects to the return_to path with user's single access token" do
    controller.should_receive(:redirect_to).with(redirect_to_url + "&token=foo")
    controller.callback
  end
end

You can see that we need a fair amount of initial setup to get both the regular methods all controllers access and the specific ones we want to test or stub in the specific tests. It's still feels reasonable considering that we lose the Rails loading overhead and tests are incredibly fast.

Here is the controller:

def callback
  if params[:code]
    cc_user = connect_user_to_salesforce
    redirect_to Crowdcast::UrlGenerator.url(:url => return_to_url_from_callback, :params => { :token => cc_user.single_access_token })
  else
    flash[:error] = params[:error_description] if params[:error_description]
    redirect_to return_to_url_from_callback
  end
end

private

def connect_user_to_salesforce
  SalesforceConnection.connect(existing_or_autoregistered_user)
end

def existing_or_autoregistered_user
  current_user || Salesforce::UserAutoRegistrar.register(current_site)
end

def return_to_url_from_callback
  state_from_callback["url"]
end

def state_from_callback
  JSON.parse(CGI.unescape(params[:state]))
end

And now a problem.

We want to add some more Rails controller goodness in case there are exceptions (they're always where you least expect them). Check this out.

rescue_from Salesforce::UserResolver::UnsupportedAccountTypeError, :with => :account_error
rescue_from Salesforce::TokenResolver::AuthCodeExpiredError, :with => :expired_auth_code

def account_error
  render :status => :precondition_failed, :action => "account_error", :layout => "not_found"
end

Now we need to figure out how to get rescue_from or have more stubbing on the controller class. Before, when our controller was very lightweight, we could deal with the minimal amount of manual dependencies to get the speed increases. But at this point we decided to convert the controller to our "slow specs" folder, ie, our regular /spec folder, since the pain of managing the external dependencies reached a threshold we weren't willing to cross.

How did we decide this was the time? It wasn't anything specific but the overall feel of the code getting too complicated and stub-happy; we weren't getting pain from bad design but from using a framework.

Conclusions

Testing without Rails is still new, and we're still learning what works and what doesn't. Should we automatically test functionality that's coupled to the framework within the framework? I still say no, that we can get out of the framework if we use only a minimal subset that we can maintain manually. We decided to return to loading Rails when that subset was no longer minimal. That situation did not come up for some time, and it isn't a foregone conclusion that it always will. It's a developer's decision on the tradeoffs. Plus it was a great learning experience.

Friday, May 25, 2012

Reading: The Developer's Code

I recently finished reading The Developer's Code and found some ideas that really resonate. There's a lot in the book that feels very obvious, but then I realize how long it took me to learn those lessons and how many mistakes I've made to learn them. Those thoughts make me realize that the other parts of the book, the ones that don't click as much, are probably even more valuable since I now have an opportunity to learn things without the pain of experiencing then through failure (although I'm sure I'll still get to enjoy those feelings quite often).

Essay 11 is titled Stop Programming. Its argument is that it's not about the hours one puts into the work but the quality. "Great programming is about maximizing the time you’re working at your best, not the cumulative hours you spend in front of a screen." This is readily obvious from thinking about charging clients an hourly rate vs. a fixed rate. Does it really matter if I sit at my computer for eight hours if I can provide just as much value from two hours of work? Won't work find a way to take the entire time allotted unless we're ever-vigilant? The difficulty for people to realize this is that programming is abstract and value isn't guaranteed through lines of code written.

The next essay, Test Your Work First Thing in the Morning, posits that we're mentally and physically fresher earlier in the day, so it makes the most sense to do the hard things then. By the late afternoon, "our perception of what makes sense or feels right now competes with fatigue. Also, fatigue makes us miss the small details." If we haven't stopped programming to take a break or otherwise recharge, our cognitive abilities will deteriorate more than normal. Thinking as an activity is difficult.

Pair programming is wonderful, but it's intense as well. We've come to understand that we shouldn't work on the more difficult problems at the end of the day, so we always try to leave small bugs or simple features for that time. It's also motivating to start the day solving more interesting problems rather than putting then off until the motivation has waned. At that time it's better to just get something done that is low impact.

Monday, March 5, 2012

Don't be lazy with route resources and allow unused ones

Refactoring controllers to make then skinny is usually a multi-step affair. The common process is to get current functionality tested and them refactor the business logic where it needs to go, moving the tests along with them to their corresponding test files. As the code is moved (to models or service objects, etc.), we can stub out those calls in the controller. Eventually, hopefully, the controller actions will be only a few assignments and a render call. Since the effects can reach across the entire Rails architecture (the M, V, and C, if you will), I find smoke testing often a necessary step in the process.

Which brings me to two sources of frustration: the default Rails (2.0) routes and default restful routes.
map.connect ':controller/:action/:id.:format'
This is just ripe for annoyance. Having a list of all the routes set up and not being able to find the one that I need? This guys fault. Rails has a lot of magic, but this has become a major pain because of the implicit and non-obvious nature of allowing anything one wants to sit next to a specified list. But then again, that specific list can have problems of its own too.
map.resources :photos
This can be fantastic. One line gives us so much power and ease: 7 actions for the price of one. What's the problem? Well, when I was recently working on some tech debt, I thought I found some unused actions. But how did I know for sure? I had to do a lot of grepping and manual testing to make sure these really weren't used. It turned out that one I thought wasn't was actually called in one specific case, so I had to back out my changes.

This would have been less painful if the code coverage was better, but that's sometimes the nature of old code, isn't it? If there are methods in the controller that aren't used, it's one thing to not have them declared, but what happens when there is code for an action that goes unused? I decided to be explicit about which routes to use so I didn't have to deal with the same problems later on (I can be my own worst enemy sometimes).

Let's lock down those resources:
map.resources :foobars, :only => [ :index, :show, :new, :create ]
If we're not using :edit, :update, and :destroy, let's not clutter things up with implications.

Maybe it's overkill, but tech debt can be painful to pay back, and I don't want to piss off my future self if this were to happen again.

Sunday, February 12, 2012

Vimprovements

After the improvements I made to my Vim setup, I realized I could get that setup in a repeatable state without too many problems. Here's what I did.

The first decision was to not use Janus. I enjoyed my time with it, but I wanted to have full control over Vim and have a reason for each new piece of functionality. I went through the .vimrc and .gvimrc files and the list of plugins and took only the ones I used for day-to-day development. If I have a pain point and want more functionality, I'll find a solution organically, but I didn't want to have a kitchen sink setup where I didn't know if the functionality is Vim's or Janus'.

Don't feel that this means I dislike Janus, as I think it's a great environment, especially for those new to Vim, but it's time to move on.

I then installed pathogen for easy plugin management. I used submodules to install all the plugins I wanted so I'm able to easily update them through git. The two non-trivial installations were Command-T and javaScriptLint. I included instructions in the README for getting those installed. For the latter plugin, it's a nice thing to add the following line to your .vimrc file to get rid of the rather strong highlighting color:
let g:jslint_highlight_color=""
To get the .vimrc and .gvimrc files in the same folder, I renamed them without the dots and created symlinks with the original names in the root directory that pointed to them. Now everything was under version control and can go on whatever machine I want.

You can find my setup on github. Feel free to take what you like.

Monday, January 16, 2012

State Machine Transactions

The Setup


We have a model that has a state machine on it to activate, close, and cancel after meeting certain conditions. When closing this model, we want to manipulate an associated model as well, and we want these in a transaction to preserve data integrity. Unfortunately, there is logic in the associated model that doesn't like to have the original model in the new state before saving. The obvious solution is to set up the following callbacks:
state :closed, :enter => :enter_close, :after => :after_close

def enter_closed
  # do stuff to model
end

def after_close
  # do stuff to associated model
end

The Problem

It turns out that the callbacks are not wrapped in a transaction and that #after_close is called after the model saves, leaving the associated model in danger of getting in a back state or failing validations.

The Solution

Using this post as a guide, we ended up with this code:
class ActiveRecord::Base
  def self.transaction_around_transitions
    event_table.keys.each do |t|
      define_method("#{t}_with_lock!") do
        transaction do
          send("#{t}_without_lock!")
        end
      end
      alias_method_chain "#{t}!", :lock
    end
  end
end
And call that method after setting up the transitions:
event :close do
  transitions :from => :foo, :to => :bar
end

# other transitions...

transaction_around_transitions
Now there is a transaction around the entire state change and its callbacks, and we'll be able to sleep tonight knowing that all the saving is safe and sound, all wrapped up in a nice, warm transaction.

Thursday, December 22, 2011

Can We Have Too Many Tools?

Vim is a great editor. There are so many plugins that make it even better and increase my productivity. But can there be a saturation point where it's not worth finding the next plugin to shave a keystroke off of a command? I've been trying to find the sweet spot for the right amount of tools for the job.

Bigger Toolbox

Having more developers to talk to and work with, I have been exposed to different methods of development. Yan in particular, has been a tremendous Vim resource. He's made it his focus to optimize working with this editor as much as possible, and it's quite impressive. I've adopted some of his ideas, no longer content to be "good enough" with Vim.

Janus has been a tremendous help. It exposed me to command-T, which is now my favorite thing ever. I no longer use a buffer explorer because it's easier to just find it with command-T than search through the buffer list. I also rarely use NERD tree now, since I was using that mostly as a convenience to open project files. I'll still use it for looking in directories, but it's not open by default anymore.

From Yan, I've install git-grep and mapped K to search for the word under the cursor, and that's been such a pleasant, and faster experience, than using vimgrep. Yan is also cleaning up a plugin for rspec integration that provides some nice wins.

Too Big?

But how far do I take this path? I could continue to add plugins and map commonly used keystrokes to further increase efficiency, but when do I start to get diminishing returns? For example, I could map a letter to :GitGrep and save six keystrokes, but I haven't found the need to do that yet. Usually when I'm searching for something, I'm thinking about what I want to search for while typing the command, so I'm effectively multitasking and not wasting time with all that extra typing. Sometimes slowing down can be a good thing to allow that planning. Plus each new mapping or plugin is something new to learn, and it can occasionally become overwhelming with all the new options.

Just Right, For Now

I'll definitely continue to improve my Vim Fu, but I'm not in a hurry to continually add to my repertoire until I've mastered what I currently have available. I've come this far on a basic setup, and Vim has been around long enough that there have been many others who have gotten by with less, so I'm not going to stress out about not optimizing every single key I type.

Sunday, December 4, 2011

Pair Programming Feedback

It's been a few weeks of pair programming with the new guys, and it's been so enjoyable and valuable. Here's what I've experienced.

Impproved Toolset

Since there are multiple people working on the same computers, we need a shared toolset. Some of us like RubyMine, while others prefer the awesomeness of vim. Since everyone prefers one of these two, we can set them both up on each machine and switch editors as needed. Yes, it would be optimal for pairing to use the exact same environment and setup like Pivotal Labs, but you can pry vim out of my cold, dead hands (hard drive?).

It's easy to keep ones toolset static after getting used to that way of doing things, so getting others who have different processes allows us all to question our own and change those parts that are lacking. I've finally come around to Janus, a suite of vim customizations and plugins, and it's been great. Command-T is almost worth it by itself. Yan also introduced me to git grep, which has been much faster than vimgrep.

Improved Code

The value of the discussion-aspect of pairing becomes immediately obvious when there's a disagreement. Trying to argue ones position on a subject exposes how much one really understands about that subject. If I can't explain why we should use technique x to my pair, do I really know enough to say for sure that we should? It's a great didactic tool for all parts of the development process - pretty much anything that we would write is subject for debate, and the discussions are invaluable for coming up with the best solution.

More Fun

Having more people in the office has made the place much more lively and interesting. Since we're such a small startup, any new person is going to have a major impact on the physical space of the office and the dynamic of the group (going from four to five is kind of a big deal). Our new interview process seems to have worked because it's been a pleasure working with each new team member, and everyone gets along with everyone else.

Exhaustion

Pair programming is fun, but it's tiring to work so intimately and intensly with someone all day. The productivity has gone up a lot because of the focus, the breaks have become more separate (not just reading an article on the computer but getting up and moving away from the damn thing), and I'm definitely ready to stop coding by the end of the day. It's a good exhaustion, a feeling that much was accomplished and comes with a certain buzz that's different than when working by myself all day. It's a new stress, but I'll adapt to it and get even better.

We're in a good place, and we're ready to continue writing great software.

Friday, November 4, 2011

Rails 2.3.5 Bug With accepts_nested_attributes_for

The Setup

At work, our application has a series of questions it asks users, and an administrator can set a target of what he thinks (or wants) the answer to be.
class Question < ActiveRecord::Base
  has_many :targets
  accepts_nested_attributes_for :targets, :allow_destroy => true
end

class Target < ActiveRecord::Base
  belongs_to :question
end

The administrator fills the form out, choosing a target or not, and Rails will save, update, or destroy it as normal. Well, almost. Apparently there is a bug in Rails 2.3.5 that will save the child model twice, and we're experiencing that now.

What I Want To Do

Upgrade to Rails 3.1, get all the new hotness and bug fixes, and not worry about this problem.

What I'm Going To Do

Since we don't have the resources to upgrade Rails (yet...), we need a workaround. Here's what we came up with.

after_save :ensure_one_target_while_in_draft

def ensure_one_target_while_in_draft
  if draft? && targets.count > 1
    targets[1..-1].each { |t| t.destroy }
  end
end

This problem only comes up when a question has not been activated and is still in draft mode. Once it's activated, the administrator cannot change the target but only add additional ones. So while in that state, and if there are multiple targets, destroy all but the first. This needs to be in an after_save callback because the double-save does not happen until after the question is saved. It's not elegant, but it works. I don't want to try to patch Rails and remember to deal with that later when we finally do upgrade, and the situation is isolated enough that there isn't a performance hit and it's easy to remove once it's no longer necessary. Elegant? No. But it's a bug fix for the framework, it's well documented in the code, and it works.

Saturday, October 22, 2011

Rails Security Refactor: Protect Those Attributes!

Rails in the enterprise is still a fairly new concept, but the same web development principles we have also exist in this new realm. One no-brainer is security, and one no-brainer part of security is protecting data from malicious user input.

The basic Rails ways to protect attributes on a model are attr_protected and attr_accessible. If neither are set, it's easy to imagine a situation where a user updates his attributes and the corresponding model has a boolean admin field on it. The user can trivially submit post data that looks like this:
params: { "user" => { :email => "foo@bar.com", :first_name => "Joe", :last_name => "Hacker", :admin => "true" } }
Oops! Now Mr. Hacker is Mr. Admin!

So how do we get from here to (more) secure? Throwing attr_accessible on the model to whitelist is the safest, but it can cause a lot of unknown breakage if there aren't tests around the fields, which there probably aren't because why would someone test the fields for accessibility if they are automatically accessible? An interim step is to create a blacklist using attr_protected to only protect specified fields, get tests around these protected fields, and then upgrade to attr_accessible.

For our user model, let's protect that admin field:
Class User < ActiveRecord::Base
  attr_protected :admin
end
And the tests:
describe User do
  describe "with protected fields" do
    context "including admin" do
      let(:user) { Factory.build(:user, :admin => false) }

      it "cannot mass-update" do
        user.update_attributes({ :admin => true })
        user.admin.should be_false
      end
    end
  end  
end
We instantiate a user object with admin set to false, try to update that field, and ensure that it did not get updated. If we use user.update_attribute(:admin, true), the test would fail because that skips all the ActiveRecord protection, so we use user.update_attributes(). Doing this for all the fields we want to protect will eventually get us to the point where we can swap out attr_protected with the easier-to-deal-with attr_accessible. Since we need to be explicit with attr_protected, it can get to be difficult to maintain quickly since we need to remember to add each new field we want to protect to the list and test it.
Class User < ActiveRecord::Base
  # attr_protected :id, :admin, :awesomeness_rating, :money
  attr_accessible :email, :first_name, :last_name
end
That's better. Tests and iterative development to the rescue!

Thursday, September 15, 2011

Testing controller JSON responses in Rspec

I was recently rewriting some controller specs because they were way too heavy: all the models were saved to the database and there was no mocking. While trying to test the JSON response of an action, I got the following exception:
ActiveSupport::JSON::Encoding::CircularReferenceError
Here is the relevant controller code:
format.json do
  render :json => as_json(@questions)
end
Here is the test's mock:
question = mock_model(Question)
A little cryptic, right? After a little digging, I changed the mock to this:
question = mock_model(Question, :as_json => {'foo' => 'bar'})
Ah, there we go!

The spec ended up looking like this:
context "as json" do
  it "lists the questions" do
    question = mock_model(Question, :as_json => {'foo' => 'bar'})
    Question.should_receive(:find_ordered_subjects).and_return([question])
    get :index, :format => 'json'
    response.body.should == "[{\"foo\":\"bar\"}]"
  end
end
In our application_controller.rb, an as_json() method called to_json, which would call as_json in the test, resulting in a circular reference. Oops.

Don't forget to stub as_json()!

Saturday, August 27, 2011

Emailing with Delayed Job

We used Delayed Job to queue emails sent out to users, both to offload that blocking process and for scheduling. It has worked well so far, but recently there were some strange bugs popping up. Some emails were stuck in the queue, and the error message was about bad YAML syntax.

Delayed Job serialized objects in its handler field, and, with some user input that's not encoded properly, created incorrect YAML. For example, this could happen:
id: 1
  foo: 'here is some 'text'
  bar: 'something else'
Notice the odd number of single quotes in foo? Yeah, that's bad. Since we already had that kind of data saved, we needed another way to fix this.

Instead of having methods in the notifier.rb file like so:
def forgot_password(user)
  ...
end
We did it like this:
def forgot_password(user_id)
  user = User.find(user_id)
  ...
end
Delayed Job serialized just the user id and not the entire user object, so any potentially harmful data wasn't saved. This was more expensive since the objects had to get instantiated again, but sending out email wasn't expensive for our app, so this solution worked well.

If you ever get strange YAML syntax errors from delayed job, perhaps this method will work for you.

Friday, August 12, 2011

Attaching Events to a Disabled Submit Button

There was a form that had a few required fields, and I wanted to show a message when the user hovered over the submit button when not every field was completed. The problem was that the submit button was disabled until the fields are filled in, and I couldn't attach an event to a disabled form element.

One solution is to add an invisible element over the button.
var $disabledSubmit = $('#submit_wrapper input:disabled');
var $disabledSubmitParent = $('#submit_wrapper');
var $overlay = $('<div />');
$overlay.css({
  position: 'absolute',
  top: $disabledSubmit.position().top,
  left: $disabledSubmit.position().left,
  width: $disabledSubmit.outerWidth(),
  height: $disabledSubmit.outerHeight(),
  zIndex: 10,
  opacity: 0
});
$overlay.mouseover(this.submitHoverOver);
$overlay.mouseout(this.submitHoverOut);
$disabledSubmitParent.append($overlay);
This created an overlay over the button that handled the hiding and showing (submitHoverOver()/submitHoverOut()) of the message.

When the form was ready to submit and the button was enabled, we needed to do two things. The first was to lower the z-index of the overlay so the user can access the button.
$overlay.css('z-index', -1);
The second was to unbind the events on the overlay so we didn't continue to show the message.
$overlay.unbind();
If the user changed the data to be in a bad state, we disabled the submit button. We also needed to reattach the events and crank up the z-index of the overlay.
$overlay.css('z-index', 10);
$overlay.mouseover(this.submitHoverOver);
$overlay.mouseover(this.submitHoverOut);
Now, instead of putting up an error message or more text about required fields, the user would be directed to finish the form if he hadn't done so when he tried to submit.

Saturday, August 6, 2011

My name is Danny...and I make mistakes.

Yes, it's true. Here's what I did, and here's the reaction.



At work a few weeks ago, I was going through some callback code that sets some meta data on a model's associations. I noticed that a related flag wasn't getting set as well, that it was only set in one other specific instance with the meta data. Hmm...let's fix that, shall we? Flag added, moving on.

Star wipe to this week.

We found a bug when displaying historical data, and I quickly realized that the display was wrong because it was skipping over objects it shouldn't, objects that were flagged when they shouldn't be. Cue pants pooping.

There was a fix, and I would just need to run a script that would update the flag for all the associated models affected after the callback happens and ignore the other ones because those were the ones that explicitly have the flag set at the other, correct, time. But should I tell anyone or do I just run the script and say that I fixed the display bug? Well, WWJD (what would Jack Nicholson do)? He'd tell everyone, damn it, because he's like that. Keeping it real. Not like Chuck Norris.



Anyway, I sent out an email admitting what happened, and I included a high-level explanation of what happened along with a technical one. I explained that there is a fix and we won't lose any data, and that it would fix the current display bug but that we need to do some tests to make sure it didn't affect any other parts of the application.

My manager's response? "Hey, man, shit happens. Glad you fixed it." That's why I like working here.

Wednesday, July 13, 2011

Adding Field Separation for List Data

A Big List


MetalDetectr is effectively a list of data as specific as a user wishes to see. It will show only a list of releases a user has in his last.fm library to a list of everything on metal-archives.com. A big concern is presenting it properly, and one method is to delineate releases by whatever sort method a user wants to see. This can be by release date, by the band's name, by the release's name, or by the release's format (eg, EP, full-length, DVD).

The Algorithm


  • Start with a table row showing the earliest or most recent, depending on sort order, of the selected sort column.
  • Loop through the releases.
  • If the current release's relevant field is greater/less than the preceding one, show another table row with the current release's field value.
  • Show the release.

For example, the default sort is by US release date starting at the earliest date (and the current month so there's less noise). The list will display the current month and every album released during that month. When a release is next month, it will show next month and then all releases from that month. Continue on through the rest of the releases. If the user wants to see the list in descending order, it will show the last month first and work its way to the most current month.

The Code


First find the first value and display it in a full column span table row:
# views/releases/index,html.haml
- comparison_value = @releases.first.chain_methods(Release::FIELDS_WITH_METHODS[Release.default_sort(params[:s])])
= separator_row(comparison_value)
These two lines use the following methods:
# models/release.rb
# Sets the sort order to what's passed or us_date.
def self.default_sort(sort)
  sort || 'us_date'
end

# models/release.rb
# Takes an array of symbols and calls them on the release instance if it
# responds to them.
# Example: release.chain_methods([:us_date, :month]) => release.us_date.month
def chain_methods(methods)
  methods.inject(nil) do |memo, acc|
    target = memo ? memo : self
    target.respond_to?(acc) ? target.send(acc) : memo
  end
end

# helpers/releases_helper.rb
# Creates a row with a full colspan for the value.
def separator_row(value)
  value = Date::MONTHNAMES[value] if value.is_a?(Fixnum)
  content_tag(:tr, :class => cycle('even', 'odd')) do
    content_tag(:td, value, :class => 'separator_row', :colspan => 7)
  end
end
FIELDS_WITH_METHODS is a constant that contains a mapping of field names and methods to call on them to display properly:
# models/release.rb
FIELDS_WITH_METHODS = {
  'band' => [:band, :first, :downcase],
  'name' => [:name, :first, :downcase],
  'us_date' => [:us_date, :month],
  'euro_date' => [:euro_date, :month],
  'format' => [:format],
  nil => [:us_date, :month]
}
Then we loop through each release, updating the comparison value when we get to the next one:
# views/releases/index,html.haml
- @releases.each do |release|
  - current_value = release.chain_methods(Release::FIELDS_WITH_METHODS[Release.default_sort(params[:s])])
  - if Release.values_compared?(current_value, comparison_value, params[:d])
    - comparison_value = current_value
    = separator_row(comparison_value)
  - else
    - comparison_value = current_value
  = render release
Compare the two values based on the sort order:
# models/release.rb
# Sets the comparison operator to be greater than if the direction is nil or ascending,
# or less than if the direction is descending.
def self.comparison_operator(direction)
  (direction.nil? || direction == 'asc') ? :> : :<
end

# models/release.rb
# True if both value and comparison exist and
# if the direction is ascending:
#   true if value > comparison, false otherwise
# if the direction is descending:
#   true if value < comparison, false otherwise
def self.values_compared?(value, comparison, direction)
  value &&
  comparison &&
  value.send(
    Release.comparison_operator(direction),
    comparison
  )
end
We tried to abstract the comparisons and what's displayed so we can add new fields and only need to update the field-method mapping. There is always the possibility that a field is nil, since we don't always get all the possible data for every release, so chain_methods will call all the methods it can on a release instance until it finishes or returns nil. We could have chained a bunch of try()s together, but that didn't look right.

We also tried to get as much code out of the view as we could, and it can be improved, but it's okay for now.

Sunday, June 26, 2011

A Real Life Github Success Story

Github has been a real treasure for developers, and I've used it both at work and for personal projects. Until now, I haven't used it to its full effect, that is, contributing.

For MetalDetectr, I wanted to allow a user to filter the list to see releases from artists he had in his last.fm library. A quick search led me to this gem, only it wasn't as fully-featured as I needed.

So I forked it.

Github made this really easy to do. Soon I had the repository in my account, cloned it locally, checked out a new branch, and I was working.

The code was clean and certainly made my life easier to get what I wanted. There was a /method_categories folder that contained the methods to do API calls to get or create information for artists, tracks, and users. I wanted to read in a user's library of artists, so I simply modeled this after the other files.

class Lastfm
  module MethodCategory
    class Library < Base
      regular_method :get_artists, [:user], [[:limit, nil], [:page, nil]] do |response|
        response.xml['artists']['artist']
      end
    end
  end
end
This created a get request call for a last.fm user, set an optional limit on the number of fetched results, and set an optional page number to scan to. Along with the API key, these fields are outlined in the last.fm api docs.

Testing worked similarly. A spec file contained the other method tests, so adding the following, plus a fixture of the xml response, was super easy.

  describe '#library' do
    it 'should return an instance of Lastfm::Library' do
      @lastfm.library.should be_an_instance_of(Lastfm::MethodCategory::Library)
    end

    describe '#get_artists' do
      it 'should get the artists\' info' do
        @lastfm.should_receive(:request).with('library.getArtists', {
          :user => 'test',
          :limit => nil,
          :page => nil
        }).and_return(make_response('library_get_artists'))
        artists = @lastfm.library.get_artists('test')
        artists[1]['name'].should eql('Dark Castle')
        artists.size.should == 2
      end
    end
  end
After adding these methods, I pushed the branch to my github repository and sent a pull request to the original repository. Again, github makes this trivially easy. Before it was accepted, I had this line in the Metaldetectr Gemfile:
gem 'lastfm', :git => 'git://github.com/dbolson/ruby-lastfm.git', :branch => 'library_get_artists'
With the pull request accepted and my code merged into the master branch, it looked like this:
gem 'lastfm'

That's all it took to contribute to open source software.