Tuesday, 17 June 2008

assert email sent_to recipients

We have notifications sent out to users when their orders are finished processing. Recently we added "user groups" and so orders (which used to belong to a single user) now belong to groups of users. Which brought about the possibilty that a user could be removed from the group (and therefore could no longer see the order).

It's possible that this could happen between the time they submit their order and the time that the order has finished processing. We determined that, if the user no longer belongs to the group, they shouldn't get the notification - it should go to the next person in the group.

So our original notification code had something along the lines of:

  # Prepares an email that will notify the user that his calculation is complete
  def report_complete_notification(report)
    setup_common_values(report.user)
    @subject    += 'Your report is complete'
    body[:report] = report
  end

  # Details common to all emails from us
  def setup_common_values(user)
    recipients  user.email
    from        DEFAULT_FROM_EMAIL
    subject     "Our Funky Service - "
    sent_on     Time.now.utc
    body        :user => user
  end

So first we needed to update the notifier to call an appropriate 'recipient' if needed:

    setup_common_values(report.recipient)

Using the following recipient code in the model object:

  def recipient
    # always belongs to the original owner if no group is specified.
    return self.user unless self.group
    # use same recipient if creator still belongs to the request's group
    return self.user if self.group.users.include?(self.user)
    # otherwise grab the first person out of the group
    return self.group.users.first unless self.group.users.blank?
    # no users left - who do we notify?
    raise "HELP! The report: #{self.id} belongs to group: #{self.group.id}, but the group has no users so I don't know who should receive my notifications."
  end

As you can see, we aren't yet sure where to escalate notificaton if the group is completely empty. We have several options, but we're still waiting on the customer to get back to us and tell us what they want. The most likely option is to send an alternative notification to our admins.

How to test?

So now we need to test whether the appropriate person gets the notification. To begin with, testing whether an email went to a recipient, or set of them, isn't a pretty assertion, so I've abstracted it out into its own assertion (below). This also allows me to extend it if we need to check CC: and BCC: lists later. For now we don't use them, so "to" is good enough.

  # assert that the given recipients appeared in the recipient list of
  # the given email. If "want_match" is false - assert that *none* of the
  # given recipients were in the recipient list
  def assert_recipients(eml, recipients, want_match = true)
    recipients = [recips] unless recipients.respond_to?('[]') # arrayify if single item
    recipients.each do |recip|
      match = eml.to.any? {|r| r =~ /#{recip}/}
      match = !match unless want_match
      msg = want_match ? "email should have been sent to recipient #{recip}, but instead went to: #{eml.to}" : "email should not have been sent to recipient #{recip}"
      assert match, msg 
    end
  end
  # convenience function for asserting an email had recipients
  def assert_sent_to(eml, recips)
    assert_recipients eml, recips
  end
  # convenience function for asserting an email did not have recipients
  def assert_not_sent_to(eml, recips)
    assert_recipients eml, recips, false
  end

Now we can do various tests such as:

  # note - assume mailer.deliveries is instantiated as @emails in setup.
  def test_should_send_completion_notification
    order = orders(:completed)
    
    UserNotifier.deliver_completion_notification(order)
   
    assert_equal 1, @emails.size, "should have received one email to confirm completion"
    assert_sent_to @emails[0], order.user.email
  end
  def test_should_send_completion_notification_to_new_recipient_if_user_moved
    order = orders(:completed_for_moved_user)
    # sanity check
    assert_not_equal order.user, order.recipient, "user has moved group, but they are still showing up as the recipient"
    
    UserNotifier.deliver_completion_notification(order)
   
    assert_equal 1, @emails.size, "should have received one email to confirm completion"
    assert_sent_to @emails[0], order.recipient.email
    assert_not_sent_to @emails[0], order.user.email
  end

Monday, 9 June 2008

Rails Gotchas: Missing host error in unit tests

I finally got around to adding nice "and if you'd like to see your newly-created report, go to the URL: BLAH" links into my notification emails... a tiny nice-to-have that wasn't high on the priority list.

Suddenly my unit tests started spewing errors of the form:
ActionView::TemplateError: Missing host to link to! Please provide :host parameter or set default_url_options[:host]

The doco on default_url_options says you can add the "host" param on ActionController::Base, but no amount of monkey-patching in config/environment.rb seemed to do the trick.

Googling found a link talking about re-writing your URLs and it mentioned the problem is mainly seen in unit testing, and gave a couple of lines for a fix (as below)... but didn't say where to put them.

  include ActionController::UrlWriter
  default_url_options[:host] = 'localhost'

Experimentation showed that if you put them in your Notifer model (eg user_notifier.rb) they work just fine. Otherwise it just doesn't get into ActionMailer. :P

Tuesday, 27 May 2008

Inflector::controllerize

Ok, so I'm bored. After Ben's suggestion for improving POST redirections I decided to implement a "controllerize" inflector ;)

Not sure yet how to get this into String (so I can do: "user".controllerize). I tried going via class String, module Inflections, and via module CoreExt with no joy... so for now you still have to invoke it via Inflector. I welcome suggestions, but I really should stop distracting myself with nifty metaprogramming. ;)

Anyway, this is the code - stuff it into somewhere like environment.rb:

module Inflector
  # turns a given class name into the properly capitlkaised controller name.
  # This can be fed to "constantize" to get the actual controller by this
  # name
  def controllerize(word)
    # make sure it's the raw class name, camelize then add controller 
    "#{word.demodulize.pluralize.camelize}Controller"
  end
end

Examples of use follow:

>> Inflector.controllerize("user")
=> "UsersController"
>> Inflector.controllerize("my_widget")
=> "MyWidgetsController"
>> Inflector.controllerize("String::Thingy")
=> "ThingiesController"
>> ctrl = Inflector.controllerize("user").constantize
=> UsersController
>>

Link: 21 Ruby Tricks

Not an article, just a link to a good one. Peter Cooper has written a nice article named 21 Ruby Tricks You Should Be Using In Your Own Code. They're pretty neat! Most of them had me nodding along, but there were several that made my eyes widen in appreciation.

Wednesday, 21 May 2008

Rails gotchas: more TzTime

While I'm neck-deep in timezone land, I'll also address one little problem wih the tztime plugin. While running the functional tests I began getting errors like the following:

NoMethodError: undefined method `tzinfo' for #<TZInfo::LinkedTimezone: UTC>

I could fix it by forcing a timezone into the TzInfo object at the beginning of each failing test - but that seemed a bit of a hack. Looks like the problem has been submitted as a bug in rails ticket 11150. The suggested codefix seems to work just fine, but requires more hacking on your vendor/plugins/tztime directory. Still, it's a one-line fix, so not much to do.

The failing tests were replaced with a new error:

<Tue Apr 01 00:00:00 +0000 2008> expected but was
<2008-04-01 00:00:00 UTC>.

The failing line is in the functional test thus:

assert_equal Time.now.at_beginning_of_month, assigns(:month)

This is an instance of a problem I've seen before... ie that you don't seem to be able to compare Times with DateTimes, Dates or (now) TzTimes. The quick solution is to forcibly cast all your time-like objects into a common time-like object before testing for equality. My suggestion is to_datetime, which works for all the instances I've needed, thus the previous test case would become:

assert_equal Time.now.at_beginning_of_month.to_datetime, assigns(:month).to_datetime

Converting a TimeZone to a TzinfoTimezone

The TzInfo gem and plugins: tzinfo_timezone and tztime provide a bunch of timezone-related nifty things that intend to make your life easier by providing extra helper stuff on top of the standard Rails TimeZone class.

Personal preferences

However, I personally hate the dodgy-looking "Country/City" style of timezone list that the plugin provides. I much prefer "GMT+10:00 Sydney" provided with Ruby's TimeZone code. Especially when dealing with clients that can be expected to understand what a timezone is.

I figured I could take davantage of the niceties of TzTime/TzInfo for converting, displaying and updating activerecord etc... but use the standard Rails time_zone_select helpers on the user-preference form, and just convert the users chosen TimeZone string into a TzinfoTimezone before applying it.

But when I tried it, I suddenly started getting something along the lines of:

TZInfo::InvalidTimezoneIdentifier: no such file to load -- tzinfo/definitions/Tijuana

One big problem

Delving into the code, it seems that neither the TzInfo::Timezone class nor the tzinfo_timezone plugin's TzinfoTimezone class actually provide a method to convert from the standard Ruby TimeZone string and back... even though the plugin seems to do this internally (to generate the new list of timezones for the drop-down list).

This seems a bit of an oversight as the plugin already contains a mapping (conveniently named MAPPING) between the two.

The monkeypatch

The required code change is pretty small if done inside of tzinfo_timezone. The TzinfoTimezone class' new method tries to fetch out the appropriate timezone object, returning nil if not found. A one-line change fixes it so that after trying with the given timezone, it checks if it's in the given mapping before trying again thus:

  # replace:
  def new(name)
    self[name]
  end

  # with:
  def new(name)
    self[name] || self[MAPPING[name]]
  end

The form drop-down

Adding the field to a user is fairly simple - assuming the string column is named "time_zone" - I use the pretty standard:

<%= time_zone_select 'user', 'time_zone', TimeZone.all, {:include_blank => true} -%>

The Controller changes:

According to the tutorial on using TzTime, we put an around_filter in the controller to prep TzInfo with the logged-in user's chosen timezone (or a default if they have none, or there's nobody logged in). Becuase the plguin doesn't directly update the TzInfo::Timezone class, we have to alter that to directly call our updated method.

  # old way to set up default timezone
  around_filter :set_timezone
  def set_timezone
    # pull pref from the user - if they've supplied it
    if logged_in? && !current_user.time_zone.nil?
      TzTime.zone = TZInfo::Timezone.new(current_user.time_zone)
    else
      # otherwise use the environment's default = UTC
      TzTime.zone = TZInfo::Timezone.new(ENV['TZ'])
    end
    yield
    TzTime.reset!
  end

  # new way to set up default timezone
  around_filter :set_timezone
  def set_timezone
    # pull pref from the user - if they've supplied it
    if logged_in? && !current_user.time_zone.nil?
      TzTime.zone = TzinfoTimezone.new(current_user.time_zone)
    else
      # otherwise use the environment's default = UTC
      TzTime.zone = TZInfo::Timezone.new(ENV['TZ'])
    end
    yield
    TzTime.reset!
  end

A validation for good measure

just to add to the pile, a validation to make sure the user's chosen timezone actually exists...:

  validates_each :time_zone do |model, attr, val|
       model.errors.add(attr, "is not a valid timezone") unless val.blank? || TimeZone.all.any? {|tz| tz.name == val }
     end

Tuesday, 20 May 2008

Selling yourself tomorrow to buy today

Wow - I just read a really thought-provoking piece on The Register titled: Privacy? Forget it. Sell your brain and desires to the highest bidder

It discusses the future intentions of companies seeking to buy your purchasing habits; and provides insight into the reasons behind Phorm, Facebook's Beacon, and many of the cutting-edge market-data gathering machines out there.

Nothing new you say? So thought I until I got through it all - it's a fresh and entertaining angle on an argument that we may have heard a lot about - but which still has room for improvement. Well worth a read.

Here's a snippet to get you started:

All around us the toasters are getting smarter. Sadly, we don't seem to be keeping up with the program. We remain poor schlubs.
In the good old days, a toaster was just a toaster. It warmed bread and issued the odd electrocution. Tomorrow's toaster, however, brings with it a new set of functions that travel well beyond bread.
As we hear it, the toaster of the future will contain more silicon and communications systems. It will have sensors that can detect an upcoming failure, and alert you, via e-mail perhaps, to order a new toaster. Or maybe the toaster will order a new version of itself from the internet if you've enabled the self-upgrade feature. That future toaster may also talk to other gadgets in the home. Perhaps they'll agree on when they should shut down to save energy or to avoid a fire. Maybe they'll agree on the next brand for their upgrade cycle and make sure you get matching stainless steel appliances throughout the home or that your current penchant for pastel kit is obeyed...

Read on...

Friday, 9 May 2008

Lay a Wizard over your controllers

Adding a wizard interface to a pre-existing system

Our system requires a user to enter in a large data set and several preferences to complete their order. It all works ok, but new users can get confused when presented with a busy and convoluted screen full of scaffolds and forms - which the initial (advanced) interface currently looks like. So we decided to add a step-by-step "create your order" wizard to help new users get an idea of what they need to do in order to create, edit and submit their orders.

As it happens - the ordering (and number) of steps in the middle don't matter too much. It's all just entering random bits of information. As long as the user creates the order at the beginning and submits it at the end - it doesn't matter if they add their widgets first, or set up their preferences first. In fact, given we don't care about the specifics of the steps it makes some sense to make sure this wizard is highly flexible.

Lucky for us - Ruby gives us that luxury.

Now, we already know the system does all of the existing parts of the process - it's currently happily creating/updating and submitting orders as we speak. So "all we need" is a new overlay of views and a new path through the controllers.

But how to do that in a nicely independant way?

First - make thy controller

We'll need one of those, and we'll need an action for each step in the process - plus an index action thrown in for good measure. So lets start with:

script/generate  controller WizardController create_order add_widgets specify_preferences review_and_submit index

Adding an action per step lets us make nice, named routes that look good in the URL-bar for our user. They also make routing easy. We only need to add the route:

map.wizard '/orders/:id/wizard/:action', :controller => 'wizard', :id => nil

Note that this is great for all actions on an existing order thats saved in the db and has its own id. However, a new order won't have an id yet, so we'll need a route to cover that too:

map.new_wizard '/orders/new/wizard/', :controller => 'wizard', :action => 'index'

Here's the basic wizard controller. Note that we try to fetch out the order before each step with a common before_filter. The "index" action will dump the user into the second wizard-step if they specify an existing order id. This is so they can come back to the interface from, say, a list of orders and not get dropped (confusingly) on the "new order" page.

# controls the "new order wizard" functionality
class WizardController < ApplicationController
  # this list holds the current ordering of the wizard steps. It's used by the 
  # workflow-display code and anything that needs to know what the "next step" is
  WIZARD_STEPS = [:create_order, :add_widgets, :specify_preferences, :review_and_submit]

  before_filter :login_required
  before_filter :prep_for_step

  # the various steps in the process of the wizard
  # Note - they are named, rather than numbered so that they can be
  # adjusted/renamed/removed without worrying about their ordering.
  # There's no code as most of them all operate simply off the order 
  # (which is fetched out for every step) and have a same-name template
  def create_order
  end
  def add_widgets
  end
  def specify_preferences
  end
  def review_and_submit
  end
  def index
    # if we've found an existing order, go straight to the first data-entering step
    return render(:action => WIZARD_STEPS[1]) if @order
    render :action => WIZARD_STEPS.first # otherwise begin at the beginning
  end

  private ############################################################
  # preload the order, if an id has been passed  in - or create a temporary one if not.
  def prep_for_step
    return false unless logged_in? # to be sure, to be sure
    @order = current_user.orders.find(params[:id]) unless params[:id].blank?
    @order ||= Order.new(:user => current_user)
    @wizard_step = params[:wizard_step] # save current step to pass into templates
    true
  end
end

Make a template for each step

Each action will need an associated template showing the options available to a user for that step of the process. This template will probably import partials from your pre-existing views. After all, you're laying this interface over an existing one - so template re-use is a Good Thing. The view re-use gives similar operations a common appearance across both the wizard and advanced interfaces. This helps a user make the step from the simple to the complex interface, due to a pre-existing familiarity with the appearance.

So why not just use the same views as your existing system? There are a few benefits to having separate wizard templates.

First is to provide a unifying, common "wizard interface". Eg a visualisation showing where the user is in the process workflow. Possibly ven a subtle change in the page style to make sure a user knows when they are "in the wizard" as opposed to the rest of the site. Clues as to orientation are surprisingly helpful, especially to new users who are unfamilliar with the layout and content of your site. Every bit helps.

Second - this interface should contain extra instructions on the requirements of the current step. You know for a fact that the wizard will be used by the novice users of your system - so make sure you support their needs by providing extra help. The purpose of this wizard is to hand-hold your newbies through a difficult and complex procedure. Now that you've cut it up into bite-sized pieces, make sure you don't let them down by failing to explain each step.

Finally, the wizard interface should be much simpler than the normal view templates. As aleady stated, these users will be brand-new to your site. They don't want the advanced power-user features just yet - they just want to know how to get started. Any highly complex extras should be left out. You can keep the advanced uber-functions for when you user is confident enough to move beyond the wizard to your standard (now advanced) interface. Just keep the bare minimum that a user will need for this wizard to be functional, without crippling it so badly as to be useless. ;)

Displaying the workflow

Your user will need to know: a) what steps are in the wizard b) what step they are currently on. This is basically just a fancy tab-navigation structure, with the current one highlighted. I've covered this sort of thing in my tabbed navigation articles, so I won't repeat the details here.

You will need to store the set of steps somewhere accessible. In this case, we figured it made most sense to store the list as a constant in the WizardController itself. I also recommend using little images of right-facing arrows in between each "tab" to give the perception of flow.

I generally add a step-specific overview (eg "In this step, add widgets to your order") to get the user oriented. For us this goes directly underneath this tab-list so there's an obvious visual connection between the overview and the current-step. Don't make it long or it won't get read. Brevity gets the point across better!

Interfacing with the existing controllers

So, we have nifty templates that display what a user should be updating next - eg a list of possible widgets for the user to add to their order. So when they click on the "add widget" button - what happens next?

We already have an "add widget" action in our WidgetsController and we want to re-use that because duplicating controller functionality is just a nasty maintenance nightmare waiting to happen. So, the forms that are displayed on the templates need to point at the usual controller actions just like your original views. The problem is that when the WidgetController's "create" action is done, it's likely to send the user on to "order_widgets_path" (just like normal) - when we really want it to come back to the current step in the wizard.

So - how does the WidgetController know we want the wizard instead of the usual control path? and if so - how do we know what step of the wizard we need to pass on to? and finally, can we do all this without becoming terribly hard-coded and brittle?

Firstly, we can tell the other controller that we are coming from the wizard by passing the current wizard step in a variable in the required forms/links thus:

  <% button_opts = {:order_id => @order.id, :escape => false, 
                    'wizard_step' => 'add_widgets' } -%>
  <%= button_to('Add to order', new_order_widget_path(button_opts.merge(:id => widget.id))) -%>

This will probably require some hacking on any existing partials that have embedded forms to add a field-repeater eg:

  <%= hidden_field_tag('wizard_step', @wizard_step) if @wizard_step -%>

Now you can add a check in the relevant controller actions that tests if this step is present. If so, we need to go back to the given wizard step... but checking specifically for param[:wizard_step] is a bit nasty. What happens if we have to change how we specify that we're currently executing a wizard-step? So pull it into a common method that does a "test and redirect" thus:

In application.rb:

  # redirects the user on to the given step in the wizard process
  def redirect_to_wizard_step(the_id, step)
    return false unless current_wizard_step # allow controller to continue on as normal
    redirect_to wizard_path(:id => the_id, :action => current_wizard_step)
    return true
  end

  # Redisplays the current step in the wizard process.
  # Note: only use this if the current action was *not* successful.
  def redisplay_current_wizard_step(the_id)
    return false unless current_wizard_step # allow controller to continue on as normal
    # render template is necessary as we are likely not coming via the
    # wizard controller
    render :template => "wizard/#{current_wizard_step}"
    return true
  end

  def current_wizard_step
    params[:wizard_step]
  end

In WidgetController:

  def create
    @widget = Widget.new(params[:widget])
    @widget.order_id = params[:order_id]
    # to re-render wizard-step in view - if present
    @wizard_step = current_wizard_step 
    respond_to do |format|
      if @widget.save
        format.html { 
         # if we came here through the wizard - move on via the wizard process
          return if redirect_to_current_wizard_step(@widget.order.id)
          redirect_to order_path(@widget.order) }
        format.xml  { head :created, :location => order_widget_path(@widget.order, @widget) }
      else
        format.html { 
         return if redisplay_current_wizard_step(@widget.order.id)
          render :action => "new" }
        format.xml  { render :xml => @widget.errors.to_xml }
      end
    end
  end

That's about it - happy wizarding.

Tuesday, 6 May 2008

Rails gotchas: Data Migrations conflict with validations

So the problem is that we have some basic data that we like to preload into the database - eg an initial admin user. We created the data migration to do this and it ran fine, validated correctly and was loaded into the database for us.

Over time we added new migrations and new validations and at every step the db and code were in-synch.

Now we need to set up a new testing server. We ran the mmigrations from VERSION=0 and suddenly we get an error on the data migration along the lines of:

rake aborted!
undefined local variable or method `group_id' for #<User:0x3367bc8>

Obviously this column wasn't needed when we wrote that migration, and by now we already have the admin user in our system, so the migration hasn't been run on our latest code... until now.

The problem is that the code *now* requires a group_id for the user to be valid; but at this point we haven't reached the migration that adds the groups table to the database, along with the corresponding group_id column on the users table.

However, all the latest code is in our model, including the validation that requires valid user to have a group_id. *BUT* the group_id method simply doesn't exist yet, because ActiveRecord will only find that method by reflection once the table has the appropriate column.

We have a conundrum. We don't want to remove that validation, but we also need that data to be in the database.

So what can we do?

There are several nasty options we considered, such as adding a rescue-block around the validations and catching "just that error". Unfortunately this has the problem of hiding any future errors that we may need to fix. Allowing errors to get artificially caught, like this, can lead to them ending up in production code without us having noticed their existance. :P

We could also go back and delete the data-migration from the original file and generate a new migration for it. The new migration will be called after all the current ones and so will match match the current code. This just delays the inevitable, though. We will cimplu come up against the same problem at some future time when new validations are brought in. There needs to be some solution that will fix this for all time.

There were two reasonable solutions that we considered.

The first is to put the required data into a fixture. The fixture can be kept up-to-date with the code and therefore will always pass the validations of the present-set of code... it could then be loaded on-demand (via the console), or we could alter the original data migration to load the fixture instead of creating an ActiveRecord object. The latter has the same problems as the original issue - it will try to load in things like the group_id column at a point in the migrations that the column doesn't yet exist. The former would work - but we'd need to remember to do it every time - which means it's prone to user error.

The alternative involves a simple one-word fix... which is why it won out.

The Validations module overrides the standard ActiveRecord::save function with the save_with_validations function. This latter function takes a single, optional parameter that can tell ActiveRecord to ignore validations. ie you call save(false) and it will save the record without checking that the data is valid.

If we can assume that our own data in the data migration is valid for our purposes, then we can use this to solve the problem, as the validations will simply be ignored.

Thursday, 1 May 2008

Up or Out - the attitude for contractors

I've just read the article Up or Out on The Daily WTF. This is related to/based on Bruce Webster's article Wetware crisis: Dead Sea effect. They provide a fascinating look into the culture of the skilled IT workforce as it pertains to over-zealous attempts at retaining skilled workers.

So I got to thinking about how that applied to myself, and my past experience in IT. I've worked both as an employee and, recently, as a contractor - and I've certainly noticed the difference in attitude between the two.

I believe that contractors automatically have this "up or out" mentality. They know that their required skills are wide and varied, and that to keep ahead of the game, they need to keep their skill-level up. Staying in one job for a long term leads (eventually) to skill-stagnation. Not because the company no longer has anything to teach a contractor, but because of a case of diminishing returns.

As the article states, any skilled worker can learn a lot more by exposing themselves to new opportunities, than by sticking with a single firm. However, contractors seem to be far more aware of this fact than long-term employees (even the skilled ones). Contractors seem to have learned the lesson that current and diverse skills are marketable, and that in-house knowledge of a specific system, while beneficial to keeping a specific job, doesn't lead to expansion of a skill-set. This leads contractors to constantly push themselves to try new and different things to stay ahead of the game - to keep themselves a marketable commodity.

Employers don't seem to have kept up with this understanding. The majority of employers look poorly on hiring contractors (except for specific skills lacking in-house), compared with potential long-term employees. Contractors fall foul of the attitude mentioned in the article - the feeling that contractors are just "dating around" and aren't serious about their committment. This despite the fact that employers are no longer loyal towards employees (a job is no longer a career-for-life).

So are contracters flighty or are they simply being honest? We know that our skills improve by varied experience. Stagnating in a single shop isn't good for us *or* for the company - yet we constantly get the look-down-the-nose treatment as though we are far more unreliable than their indentured employees.

Workers and companies both benefit from fresh-blood. There is definitely a balance involved here - continual turnover can make it impossible to keep hold of institutional knowledge (which is why documenting procedures is so important!), but a company without fresh-blood will stagnate and lose market-share due to a lack of new and innovative insights coming into the company. Any firm can benefit from a good mix of the two. The fact that contractors have accepted and embraced this fact should be a sign of maturity, rather than flightiness.

I especially dislike that skeptical tone that comes with with the phrase "oh, so you're *not* interested in long-term employement". I feel I'm simply being realistic. But employers (and recruitment agencies) that employ the tone seem to imply I'm being disloyal... or committment-phobic. Neither of which is the case.

A few rare cases even seem to imply that I'm simply being greedy - shopping around for a better cash-deal, which is far from the case. While I'll take a higher salary over a lower one, I'm far more motivated by an interesting project and an opportunity to learn. That's why I'm in Rails rather than many other technologies that pay well. I would never take a higher salary just to do something that was mind-numbingly boring and, in my mind, dead-end. I'm just stoked that I get to work in a field that is cutting edge *and* well paid at the same time ;)

I have no problem with sticking around with a group that values me and that contributes to my own experience. My current contract (with SIRCA) has lasted more than a year. I fully believe that I have added value to this project. I also have learned a great deal. I have also had smaller contracts where I have provided some chunk of functionality or code-review - each of which added visible benefit to the site I worked upon, and also provided an opportunity for me to learn some new aspect of the technology I work with. This is an equal-footing relationship - where both parties receive value.

My previous employment has included roles in which I felt my input was not valued, and in which I had no mentor from which to learn. Effectively I was gaining no value from them, and they were not gaining full value from me either. I stuck around for a long time trying to help out with the project - and eventually left due to burn-out. In hindsight I should have left far earlier and moved on somewhere that I had an opportunity to grow, and that valued the insights I was able to bring. It would have been a better opportunity for me - and the company I was with could have employed somebody that they would have felt comfortable with - increasing their return-on-investment as well.

In a free market - there's no point in sticking with a relationship that is not valuable, or which provides value only to one party or the other. I won't leave for reasons of greed or lack of committment - I'll only leave if you and I are no longer gaining enough value for the professional relationship to be worthwhile.

If an employer doesn't feel that this is reasonable - then we need a good talk on the concept of "fairness".

Healthy economic relationships come about when both parties benefit. Anything else doesn't make economic sense. When the benefit to one or the other fades over time, then it should be understood that this will lead to the eventual ending of the relationship. It's simply a matter of good business practice - and shouldn't be looked on as "lack of committment" any more than moving to a cheaper/more efficient supplier.

</whinge> :)

Update: Bruce Webster has written a follow-on article: some thoughts on up or out which gives a few ideas on how to avoid developer-churn or the Dead-sea effect (and even counteract the thermocline of truth) by providing a non-managerial track for techies to follow. I completely agree. If there was a way "up" that didn't involve "out" I'd go for it!

Monday, 28 April 2008

redirect_to POST

Our current site uses the acts_as_authenticated plugin. When a user's session has timed out, we redirect them to the login page, and save the URL they were trying to reach. If they successfully login, we do a redirect to that URL. This all comes as standard in the plugin.

The problem is that redirect_to doesn't seem to preserve the HTTP method (ie GET or POST etc). Most of the time, a user will have clicked on a link, or have a link saved in their bookmarks. The URL will be something like /users/42/orders which would be a a GET-based request that is supposed to show the users current orders. However, sometimes a user will have left their browser session open while they went off to lunch, displaying their set of orders... along with a set of buttons that, say, delete or clone an order.

The problem comes in when this timed-out user tries to click on one of those buttons. A timed-out user that clicks on, say, the "Clone this order" button gets the following error: no route found to match "/user/42/orders/23/clone" with {:method=>:get}

Buttons (which are really a form-post) generally have an HTTP POST action - even the ones with Rails-faked-up PUT or DELETE actions are really POST under the covers. Our nicely RESTful application is set up to not accept GET-based requests for dangerous (non-idempotent) actions such as "clone".

Now, acts_as_authenticated uses two methods for storing/redirecting to URLs. The first is called store_location. The code here grabs the URI that the user requested, and stuffs it into the session. The second is called redirect_back_or_default - it tries to redirect back to the uri stored in the session - or to a given default.

The method: redirect_to is the Rails-standard way of sending you on to another URL via an HTTP redirect. Unfortunately, it seems that this method only tries to use GET-based URIs, so when we tried redirecting to the POST-only URI for the "clone" request, we got the routing error.

At first I thought all that was needed was to save the request's HTTP method, and pass that into redirect_to. Saving the method is easily done, and I stuffed it into the session along with the URI. Then I dug around in vain trying to find out how to pass in the HTTP method as a parameter.

Reading all the doco yielded no joy, nor did digging into the rails core code to see how it deals with uris, sessions, requests or redirection. It wasn't until I delved deep into the HTTP spec itself, and asked a few questions on RoR Oceania that I finally confirmed that it simply isn't "done" to try to redirect a user to anything other than a GET-based URI.

This is somewhat disappointing, as I don't see why a GET has to be treated like something special by HTTP - the use-case for allowing redirection to any other verb is there. :(

Still, there's no getting around this limitation, and the system must fail gracefully (which the routing spew doesn't). So the next best thing is to capture an attempt to access a non-GET request and try something different.

I figured that most of the time when a user tries to get to a non-GET request, it'll be from a button from another page on the site. eg, they'll have left their brower open on the "My orders" page, and have clicked a button from there. It's far less likely they'll somehow have hooked up a bookmark to a POST-based URI, and just as unlikely that some other site will have a button to our site. Therefore, I figued the best option is to try sending the person back to the page they started on (ie the "My orders" page), rather than the page they are asking for (ie whatever button they clicked on).

This can be solved fairly easily. If the user asked for a GET-based request we have no problem, just store it as usual. But if they asked for something else, then we need to save the referer-URI instead. I've also added a flash error to tell the user what happened, and why they have to click the button again.

So the usual acts_as_authenticated code becomes:

    # Store the URI of the current request in the session.
    #
    # We can return to this location by calling #redirect_back_or_default.
    def store_location
      # if the user has asked for a non-get request (eg posted a form). We
      # can't redirect to that - so try getting the referrer (probably the
      # index page they came from). We will post them back to their previous
      # page and warn them about what is going on
      session[:return_get] = request.get?
      session[:return_to] = request.get? ? request.request_uri : request.env["HTTP_REFERER"]
    end
    
    # Redirect to the URI stored by the most recent store_location call or
    # to the passed default.
    def redirect_back_or_default(default)
      # if the user had asked for a non-get request (eg posted a form). We
      # can't redirect to that - so we must warn the user that they will not
      # be going where they were expecting
      flash[:error] = "You clicked on a button or submitted a form, but we cannot redirect you back to that. Please try submitting again." unless session[:return_get]
      redirect_to(session[:return_to] || default)
      session[:return_to] = session[:return_get] = nil
    end

Tuesday, 22 April 2008

RailsEnvy - Rails vs...

RailsEnvy have put out a string of fantastic parodies of the famous "Mac vs PC" ads... featuring Rails vs a range of alternative frameworks...

If you haven't seen them yet, they're worth a few minutes for a giggle.

Monday, 21 April 2008

icon buttons

We're all pretty used to the CRUD-based scaffold-ful of actions on a resource index page - such things as "show", "edit" and "delete". But adding a few columns and a few extra actions, causes a swiftly increasing footprint which gets real ugly, real fast.

Buttons stack on top of one another, or they wrap in random ways, or they take up half the width of the screen and squish up all the other data.

The soluion, of course, is to make nifty little icon-buttons. But how do we make this RESTful and DRY?

fixed_button_to

Standard button_to is a wonder - it does practically everything a button needs, and you can override anything...

Well, almost - as I discovered when I first tried to make an image-based button. The "type" is set to submit every time. The following line of code is the culprit:

html_options.merge!("type" => "submit", "value" => name)

So even from the get-go, we need to monkey-patch button_to

You can override the method, or rename it to something else - whatever you like... but sadly you'll need to actually copy the whole button_to code and replace it. So I just created a newly named method. Drop the following into environment.rb (or similar) for buttony joy:

module ActionView::Helpers::UrlHelper
  def fixed_button_to(name, options = {}, html_options = {})
    html_options = html_options.stringify_keys
    convert_boolean_attributes!(html_options, %w( disabled ))

    method_tag = ''
    if (method = html_options.delete('method')) && %w{put delete}.include?(method.to_s)
      method_tag = tag('input', :type => 'hidden', :name => '_method', :value => method.to_s)
    end

    form_method = method.to_s == 'get' ? 'get' : 'post'
   
    request_token_tag = ''
    if form_method == 'post' && respond_to?(:protect_against_forgery?) && protect_against_forgery? 
      request_token_tag = tag(:input, :type => "hidden", :name => request_forgery_protection_token.to_s, 
:value => form_authenticity_token)
    end
    
    if confirm = html_options.delete("confirm")
      html_options["onclick"] = "return #{confirm_javascript_function(confirm)};"
    end

    url = options.is_a?(String) ? options : self.url_for(options)
    name ||= url

    html_options = {"type" => "submit", "value" => name}.merge(html_options)

    "<form method=\"#{form_method}\" action=\"#{escape_once url}\" class=\"button-to\"><div>" +      
method_tag + tag("input", html_options) + request_token_tag + "</div></form>"
  end
end

Those with an unhealthy familiarity with the minutiae of the Rails source will also notice that I added a respond_to?(:protect_against_forgery?) check in there too. My site doesn't use that, and it kept breaking without the check (there must be some magic place its set in a normal Controller/Helper, but it doesn't get in there from here).

icon_button_to

Next up is to write our nifty icon_button_to helper. I use the following:

  # quick and  dodgy icon path-generator
  def get_icon_path(icon_type = 'default')
    icon_name = ACTION_ICONS.include?(icon_type.to_s) ? icon_type : 'default'
    "/images/icons/#{icon_name}.png"
  end

  # generates appropriate image-buttons, given an "icon type" and the usual
  # button options.
  def icon_button_to(icon_type, icon_label, form_path, button_opts = {})
    button_opts.merge!('type' => 'image', :src => get_icon_path(icon_type), 
                       :alt => icon_label, :title => icon_label)
    content_tag 'div', fixed_button_to(icon_label, form_path, button_opts), 
      :class => 'icon_button'
  end

Note that each icon is confined within a div classed icon_button. This allows us to neatly style each button thus:

.icon_button {
  float: left;
  padding: 0 5px 0 0; /* each icon needs a small gap between */
  margin: 0;
}

Iconography

Using the above is simple. It's the same as a normal button_to., but you add the icon_type to the front... and make sure you actually have an icon named after that type sitting in the appropriate image directory. Examples follow:


<%= icon_button_to(:edit, 'Edit order', order_path(order.id), :method => :get) -%>
<%= icon_button_to(:show, 'See order', order_path(order), :method => :get) -%>
<%= icon_button_to(:submit, 'Submit order for processing', submit_order_path(order), 
  :method => :put,  :confirm => 'This will submit the order for processing. Are you sure?') if order.valid_to_submit? -%>
<%= icon_button_to(:delete, 'Delete this order', order_path(order),
  :method => :delete,  :confirm => 'This will delete this order permanently! Are you sure?') unless order.submitted? -%>

Friday, 18 April 2008

Google Geek Girl Dinner

Getting together with a whole bunch of geeky women and talking about interesting developments in IT in a toy-filled environment. That's a great way to spend the evening! Especially with speakers such as Rob Pike, Lindsay Ratcliffe and Stephanie Hannon.

Google provided the venue and catered the event. Google's Sydney office is right in the centre of town. It's a friendly, open environment full of geek toys (eg a Guitar Hero setup in the cafe) as well as wonderful and interesting people to chat with. We had a brief tour of the office, including being shown the Google rule of thumb that you should never be more than 100m away from chocolate at any time.

Then Rob spoke about the future tech trends, and a few things google is working on. Apparently there are lots of other applications in the works that he couldn't talk about, but promised they'd be amazing.

Lindsay followed with an entertaining overview of the process of bulding Customer Experience. Especially in having to translate between the very different world-views of the technical developers, the creative designers and the problem-focussed business people.

Finally, Stephanie gave a great overview of the GeoWeb - showing all the various ways in which people use mapping technology to share geographically-based information. Including such programs as Google Outreach's Crisis in Darfur project which overlays the sites of burned-out villages on satellite imagery to underscore the extent of the genocide occurring there.

I was really enthused to meet such a variety of interesting women from a wide range of technical backgrounds. Geek women are almost always a minority in the IT world. It's rare to have more than one in a development team, and so the chance to meet up to share both technical and cultural experience is a great opportunity. I suddenly feel that I'm not alone anymore. I even met one lady who shares my interest of someday going into space

GirlGeek Dinners has been instituted to support women in the IT community - to provide opportunities for women to create strong networks with others in the field, and to attract more young women into the the industry. There is A GirlGeek blog to let you know when events are scheduled, and a Facebook group if you wish to keep in touch with the others.

I enjoyed the evening and look forward to many more to come.

Friday, 11 April 2008

Button-up your actions

Google-safing dangerous actions is second-nature by now. Any action that will change the state of a resource gets hidden behind a button. Anything else is safe as a link. But a mix of links and buttons looks ugly to the user. We want a consistent-looking interface so the user knows that "clicking on one these does something to my Order". The user doesn't need to be confused by some wishy-washy interface representation of of the level of dangerousness of their click. That's one piece of information too many for somebody that just wants to Get Things Done. So our choice is:

  1. Expose our unsafe operations - and just wait for the day when a crawler comes in and deletes all our users' orders. or
  2. Hide the safe actions behind buttons - and fiddle a bit to match the Rails RESTful interface.

We're unlikely to get crawlers come in past our login screen and our users are exceptionally unlikely to crawl for a copy of the site... but un-buttoning our dangerous actions will leave us with our pants down - and that's surely gonna bite us where it hurts someday. It's not something I'm comfortable with doing. So we've opted for the latter approach.

That leaves us with making the buttons play nice with Rails' RESTful interface. Normally this is an easy operation. Just like you have a delete button that requires you to pass in the DELETE method, you can pass in the GET method for any get-oriented actions thus:

  <%= button_to('Edit', edit_user_order_path(@user, @order), :method => :get) -%>

Magical disappearing parameters

For a long while the above approach worked just fine... but then we came across an instance where we needed to add in extra parameters. We wanted to pass along a flag that tells the controller we're using the wizard interface, rather than the standard one. It lets us rediect the user to the next step in the process, rather than the usual confirmation screen. So I tried to create the buttons looking something like below:

  <% button_opts = {:user_id => @user.id, :id => @order.id, :wizard => true} -%>

  <%= button_to('Edit', edit_user_order_path(button_opts), :method => :get) -%>
  <%= button_to('Delete', user_order_path(button_opts), :method => :delete) -%>

For the POST and DELETE buttons this parameter came through fine. For the GET-buttons, however, the flag was conspicuously absent from the params hash. :(

Checking the source code gave me something like below (note: cleaned up for clarity):

<form method="post" action="/users/23/orders/42?wizard=true">
  <input name="_method" type="hidden" value="delete" />
  <input  type="submit" value="Delete" />
</form>
<form method="get" action="/users/23/orders/42/edit?wizard=true">
  <input type="submit" value="Edit" />
</form>

As you can see - they both carry the wizard flag in the URL - so there's nothing wrong with how the parameters are getting passed in at the front. But when you click on the Edit button - the flag is missing from the URL in my browser, and from the Parameters hash (displayed in the console)... so it's somehow getting dropped along the way.

Gazing deeper into the source code, you can see that the delete form still has method="post". It passes the DELETE via the hidden "_method" field. By comparison, the edit form has replaced the POST method with GET.

This seems to be the major difference. Somehow, the standard GET form isn't behaving nicely. I don't know why this bug is occurring - it could be a browser-bug, or Rails - and it really doesn't matter. I need to make buttons that am certain will work.

Luckily, we can use our knowledge of the hidden "_method" field to our advantage. The following code forces the get into the hidden field rather than the form. It works fine and the flag shows up happily on the other side.

  <% button_opts = {:user_id => @user.id, :id => @order.id, :wizard => true} -%>

  <%= button_to('Edit', edit_user_order_path(button_opts.merge('_method' => 'get'))) -%>
  <%= button_to('Delete', user_order_path(button_opts), :method => :delete) -%>

Thursday, 10 April 2008

Array.item_after(this)

There are just so many really useful functions in Ruby that every time I look I find something new and funky. So it always surprises me when I look to find something I'd expect to see - that just isn't there.

I was hoping to find an array function that returns "the next item in the list from the given one". I have a set of steps in a workflow all stored in a list. Given the current step, I want to know what the next one is going to be... so that I can send the user on to the next action from the current one.

Luckily Ruby is really easy to extend. ;)

These functions will return the item before/after the given item... or an item of any given offset from the current one. They return nil if the item can't be found in the given list or if the offset would put the index outside the bounds of the array.

class Array
  def item_after(item)
    offset_from(item, 1)
  end
  def item_before(item)
    offset_from(item, -1)
  end
  def offset_from(match, offset = 1)
    return nil unless idx = self.index(match)
    self[idx + offset]
  end
end

Usage:

>> arr = ['a','b','c','d']
=> ["a", "b", "c", "d"]
>> arr.item_after('a')
=> "b"
>> arr.item_before('d')
=> "c"
>> arr.offset_from('a',2)
=> "c"
>> arr.offset_from('d',-3)
=> "a"
>> arr.offset_from('d',10)
=> nil
>> arr.item_after('purple')
=> nil

Monday, 7 April 2008

2.0 update or not 2.0 update: Is Rails 2.0 ready yet?

I had a play with converting one of my smaller playthings... er projects over to Rails 2.0 the other day. So far I see no convincing reason to convert yet. There were some small improvements, but I came across a couple of glaring bugs that make rake test incredibly painful to use.

When I find the specific ticket-numbers again I'll include them here, but to briefly describe the issue: The bug caused the setup method not to be called (ever) in unit tests. This meant that tests didn't get their fixtures fresh after every test. The several attempts to fix this issue somehow interfered with another bug that caused the controller not to be loaded in the functional tests.

Both of these situations are so impossible to work with that I'd recommend steering clear of Rails 2.0 for a short while.

I can see it being worthwhile using Rails 2.0 for new/greenfields projects - and putting up with the bugs until the Rails Core Team figure out how to fix them. But I would seriously recommend against it for any pre-existing project - especially one that is currently in a commercial/production environment. The pain of having to convert a complete test-suite over to work around the bugs (or to convert it all over to RSpec) isn't worth the effort just yet.

Conclusion: Yes, but not yet.

Tuesday, 11 March 2008

Performance tips > indexing matters

As I've mentioned in a previous post, I've been doing a lot of performance tweaking recently. By far and away the biggest gains I've garnered have been by tweaking the SQL/DB in various ways. One of the biggest things I learned was to use indexes appropriately. This means two things:

  1. Put an index on anything important
  2. Get the field order right

So what's important?

From the perspective of an index, immportant means anything that you will need to fetch repeatedly - especially if that fetch is in a tight loop. This is one place where RubyProf is your friend. You can also take a look at the SQL queries scrolling up the screen while watching the console output to get a rough idea of the most important queries.

What about the field order?

Indexes work better if you put the "most discerning" fields before the less-discerning ones. As an example, suppose you have a (completely spurious) table as follows:

owner_name pet_name toy_name
Bob Fido Bone
Bob Fido Rubber chicken
Bob Whiskers Jingly ball
Bob Whiskers paper on string
Jane Woofy chew toy
Jane Woofy bouncy ball
Jane Kitty paper bag
Jane Kitty laser pointer

Assume for some reason it's important to have an index on all three fields above. The field that is most unique is the "toy_name" - doing a lookup on that field will get the best bang for your buck, so it should come first in your index, followed by the next-most discerning field. In the example data above it's the pet_name, but maybe you know your intended audience a bit better. If you know that a lot of people are calling their dogs 'fido' and their cats 'kitty', you might index on the owner_name. The most likely circumstance is that you'll have the owners listed in a users list and there's actually an owner_id - in which case you'd definitely set that as the second key for the index.

Thus you'd end up with:
add_index :pet_toys, [:toy_name, :owner_id, :pet_name], :name => :idx_pettoys_by_name_oid_pet

Is it really that important?

The contract I'm currently working on requires us to display reports with tens of thousands of orders and trades in them, cross-referenced against multiple calculated benchmark values. It's imperative that the indexes are set up to pull that data out of the db in the most efficient way possible.

I acheived an order of Magnitude speedup just by changing around the ordering of the indexes! This was worth it for us - YMMV

Tuesday, 4 March 2008

assert_event_logged > Getting stuff back out of the Ruby Logger

So we wanted to test whether our code actually puts things into the log when it should - eg to log important events such as "User X just got archived by User Y" or "Someone tried to login as User X but failed!"

Unfortunately, the Ruby Logger seems to be write-only. There's no way to actually get back the messages that you carefully put into it. So how to test it?

Asking around, I got a few ideas for how to go forward. The best option would be to have a separate "audit log" for special events that need to be independantly auditable. However this would take Time(tm) (of which we seem to be in short supply these days). The next best option was to use mocha... but that,, to my mind, doesn't really test the actual logger, which I want to be certain actually works the way I expect.

So I began to think... hmmm - monkey-patch.

What follows is quick and dirty... I had intended to go for elegeant, but I'm satisfied that it at least tests the actual logger object rather than having to do something dodgy by creating a temporary, second log and reading it in as a file :P

The monkey--patches

Put these into test_helper.rb somewhere up the top (not inside class test::Unit::TestCase)

# monkey patch ruby's "Logger" to make it accessible to tests
class Logger
  alias old_add add
  def add(lvl, *args)
    @last_logged ||= {} 
    @last_logged[lvl] ||= []
    @last_logged[lvl] << args[1]
    old_add(lvl, *args)
  end
  def last_logged(lvl = nil)
    @last_logged ||= {}
    return @last_logged.values.flatten unless lvl
    @last_logged[lvl]
  end
  def clear_log_messages
    @last_logged = {}
  end
end

Put this stuff inside class test::Unit::TestCase:

  # a way of actually getting the usual rails logger object
  def logger
    RAILS_DEFAULT_LOGGER
  end
 # assert that the log has received the given message at the given log
  # level. Can pass in matching message as a string or a regular expression.
  def assert_event_logged(regex = nil, lvl = nil, match = true, &the_block)
    logger.clear_log_messages
    regex = /#{regex}/ if regex.is_a? String

    yield the_block

    last_logged = logger.last_logged(lvl)

    if match
      assert !last_logged.blank?, "should have logged some message at given level #{lvl.inspect}"
      assert last_logged.detect {|msg| msg =~ regex }, "should have matched the given argument #{regex} at the given level: #{lvl.inspect} Instead got: #{last_logged.join('\n')}"
    else
      return if last_logged.blank? # trivial case
      assert last_logged.all? {|msg| msg !=~ regex }, "should not have matched the given argument #{regex} at the given level: #{lvl.inspect}. Instead got: #{last_logged.join('\n')}"
    end
  end

  # assert that this specific event was not logged
  # ie - just an alias for the opposite of event_logged
  def assert_event_not_logged(regex = nil, lvl = nil, &the_block)
    assert_event_logged(regex, lvl, false) { yield the_block }
  end

  # check that no events were logged at the given level    
  def assert_no_event_logged(lvl = nil, &the_block)
    logger.clear_log_messages

    yield the_block

    last_logged = logger.last_logged(lvl)
    assert last_logged.blank?, "should not have logged a message at the given level at the given level: #{lvl.inspect}"
  end

Here are some basic tests that will test the above assertions are working correctly... and stand as basic examples of use:

  def test_should_show_msg_logged
    msg = "forcing the logger to log"
    assert_event_logged(msg) do 
      logger.debug msg
    end
  end
  def test_should_show_message_logged_at_a_given_log_level
    msg = "forcing the logger to log"
    assert_event_logged(msg, Logger::DEBUG) do 
      logger.debug msg
    end
    assert_event_logged(msg, Logger::INFO) do 
      logger.info msg
    end
  end
  def test_should_show_msg_logged_matches_given_regex
    msg = "forcing the logger to log"
    assert_event_logged(/forcing/, Logger::INFO) do 
      logger.info msg
    end
    assert_event_logged(/oogley/, Logger::INFO, false) do 
      logger.info msg
    end
  end
  def test_should_show_msg_logged_matches_given_regex
    assert_no_event_logged() do 
      # do something that doesn't log anything
      three = 1 + 2
    end
  end
  def test_should_not_match_msg_that_doesnt_match
    assert_event_not_logged("yada yada", Logger::INFO) do 
      logger.info "blah blah"
    end
  end
  def test_should_not_match_msg_logged_at_different_log_level
    assert_no_event_logged(Logger::INFO) do 
      # do something that doesn't log anything
      three = 1 + 2
      # log something at a log level different to that specified
      logger.debug "should not get this one"
    end
  end

PS - if anyone wondered where I've been for the past months I've been pretty busy performance-tuning the site in preparation for some sort of slightly-more-official-launch-than-just-to-special-customers-only...

So haven't had much chance to actually do anything interesting, but hopefully we're actually *that* much closer to really actually launching the site to Real People (tm).

Thursday, 31 January 2008

Rails gotchas: 1.months.ago isn't

A colleague caught this bug when we were running rake test today (Jan 31st). Suddenly the quota-calculation tests were acting weird. All our tests ran fine yesterday, so we were both pretty perplexed by why they'd suddenly stopped working today.

Quota is calculated based on a calendar month... so he delved into the fixtures looking for dates. What he came up with was a short script/console session showing the results below:

>> Time.now
=> Thu Jan 31 03:11:55 +0000 2008
>> 1.months.ago
=> Tue Jan 01 03:12:02 +0000 2008

Clearly "months ago" really isn't - it's actually "30 days ago". This means that our tests will be working again fine tomorrow, just as they have been all month... and only failed because (like so *few* other months in the year), we have 31 days in January, and this is it.

The fixtures in question had something similar to "save these quota-eating things as happening one month ago and check they don't eat into this month's quota too". And yet today, "one month ago" was being evaluated as Jan 1st... :P

With experimentation, the following does work correctly, even across funky boundaries:

>> Time.now.months_ago 1
=> Mon Dec 31 03:12:09 +0000 2007
>> date = Time::parse "March 31 2008"
=> Mon Mar 31 00:00:00 +0000 2008
>> date.months_ago 1
=> Fri Feb 29 00:00:00 +0000 2008

Tuesday, 18 December 2007

Rails gotchas: All the fixtures in the world

After having spent three hours debugging an error that "couldn't be happening" I have decided to just start loading every fixture for every functional test... Ok maybe not - but that's what it feels like is necessary.

The test in question was there to check if a user with expired quota could start using a feature again once they'd been given more quota. It was failing because I hadn't included the "roles" fixture... vital to the controller when deciding if the user is authorised to get into the controller in the first place... but not really directly relevant to this test, so I hadn't thought about it.

This can also cause the weird effect where an entire functional test suite runs fine when you call it via ruby test/functionals/<whatever>.rb, but several tests fail when you try to run it via rake test:functionals

Lesson: include every fixture that might be accessed by your controller in the performance of its duty - even if it doesn't seem directly relevant.

Friday, 14 December 2007

The Economics of IT salaries

I have been reading a book called "The undercover economist"[1], which mentioned the problems with "asymmetric information" in negotiations. It's discussed on pp112-115, if you want to look, but to summarise, the problem is thus: A 2nd-hand car salesman is trying to sell all their cars - whether they are peaches or lemons, and wants to get the best price for each. The buyer is not willing to pay as much for a peach as for a lemon. The salesman knows if any given car is a peach or a lemon, but the car buyer does not. This is the "asymmetric information".

The salesman will, of course, say "all my cars are peaches" (especially if they are not) and to try to charge accordingly. The buyer must then decide if it worth the risk of potentially paying peach-prices for a lemon. If there is no accurate way of telling one car from another, they will often choose not to buy at all.

The problem is that even if the car really *is* a peach - the customer will still not be able to believe the salesman... after all, he'd say that if it was a lemon too.

In this case, both the salesman and the customer lose out, because the salesman is in command of information that the customer is not (ie the "peachness" of the car in question).

The only way out of it is for the salesman to offer undeniable proof of a car's "peachness". The customer can then trust that the car is worth the price - and is more willing to buy.

In Australia, this is done with independant inspections, (eg done by the NRMA). A customer can be fairly certain that a car that has passed inspection meets a basic level of quality. Doing these tests takes more time that the average car-buyer has (especially when they have so many to look over), but it's worth the time for the salesman to get it done, as they then have undeniable proof of peachiness.

But it's relatively easy to verify the quality of car: does it, or does it not have working brakes? Well, test the brakes and see if they work!

What if it weren't so easy - with a set of skills that are subjective, and tests that are at best indirect, and at worst misleading. For example, how do we tell if one IT-worker is more skilled than another? How do you measure skill in IT? and who's doing the measuring? In fact, who is the buyer in this kind of transaction?

Measuring your l33t sk1llz

In the first place, we all know that it's pretty hard to tell how good one programmer is from another. Sure, it's sometimes pretty easy to assess ourselves relative to one another. Obviously David Hanmeier Hansson is easily more skilled than my kid sister. He's certainly better than I am... but am I better than my colleague in the next cubicle over?

I like to think so and I'm pretty sure that my paycheck says so too... but I'm only guessing that... and is a paycheck a reasonable guide anyway - or is that just a cue for a feedback loop?

A year ago I was working for considerably below the average salary[2]. Partly it was because I valued job-security (and thought I had it[3]) but I've been trying to figure out the other part. Why is it that companies with certain types of management don't offer more to their IT workers? Contrariwise - why is it that some companies hire idiots and pay them huge sums of money?

There was a Paul Graham article that explains that the average PHB can't tell if a technology is really good or not. They simply don't have the requisite experience to make an educated distinction.

By corollary, they also can't tell if the technology skill of their workers is really good (after all, if they don't know how to use it, how can they tell if *you're* using it to the best advantage?). The only clues they have to go on are the skills that they can see and judge from their own experience. These tend to be more socially oriented: is the worker well-dressed, articulate yet polite, well-behaved, do they show due deference to the PHB, do as they're told etc. The average PHB can measure these hirself, so they get put to the top of the list of required "professional" skills.

The skills mentioned above may well be a good indicator of whether or not they'll get along well with the manager, but they are clearly not a good indicator of skill in IT.

Now, I'm not saying these skills aren't important. Doubtless, most of these have their importance in a workplace, especially where one must get along with peers. Without any level of skill in the above list, friction will eventually ensue... and yet the original point here was not to tell how good an IT worker would be at making friends in the workplace, but how good they'd be at being an IT worker... and surely actual IT skill should rank up there as an important part of the job requirements.

Now, a good, skilled techie may know reasonably well how they stack up compared to other techies (s)he's worked with - at least within a few degrees of freedom). They tend to know that if they hand a tough task to Jo she'll get the job done quickly and elegantly, but hand it to Sarah and she'll need a fair bit of hand-holding so you'd better give her the easier tasks while you get on with the "real work".

A less-skilled worker, by contrast, may not have a clue how good they are, and you can't compare at all if you've never worked with a person[4]

So maybe they can tell, roughly, by contrast, their own level of skill - maybe it's not accurate, but they've probably got some idea of where they think they stand. In any case they have information that the PHB doesn't have... and, come review time, every worker will say they are a peach - especially if they're a lemon.

And those that have better social skills will be more convincing to the PHB.

What does the PHB think?

Now, a manager that was actually skilled in IT (eg one that had "come up through the ranks") might have a chance to judge accurately. This neatly slams up against another perpetual problem of the IT trade: that if you promote your best IT workers into management - you are wasting the IT skills that made them good in the first place. Not to mention the fact that to be a good manager requires a completely orthogonal skillset to being a good IT worker.[5].

Of course, if promotion is based on IT skill - you're in a chicken-egg situation. How do you judge good skill to promote if you cannot trust the skill-judgements of those around you? Paul Graham calls this "the design problem".

What does this mean for an organisation?

In my experience, the problems from asymmetric information are amplified in monolithic IT organisations (eg the IT departments of banks), which nurture this kind of manager - and also seem to nurture a higher percentage of mediocre programmers.

If a manager cannot tell if their workers are skilled or not - they aren't prepared to pay as well - after all, the worker might really be a lemon, just saying they are a peach. But a real peach may know that they are - and thus won't be prepared to take the low pay... so they have no choice but to leave. Everyone loses.

This is a self-perpetuating cycle. I don't know how it all started, but you can see that it'd be nigh-on impossible to break out of. A mediocre manager would not be prepared to offer competitive pay to his employees - as he's simply not prepared to pay peach prices on the offchance of buying a lemon. This causes all the actual peach epmployees to leave - they know they can get better pay elsewhere... leaving only the mediocre programmers (and lemons) behind in the monolithic organisation.

Which leaves only mediocre pickings for employees to rise through the ranks to become mediocre managers themselves one day... or the hiring of managers, but without the benefit of a sanity check on IT skills from the employees. In any case leading to generally the same cycle - bad managers, bad employees - lemons all round.

Thus the monolithic organisations fail to get or keep good staff, and the only place to find them is in the small, agile development houses, or in contracting - where a peach can have a chance of negotiating more directly with clueful people.

Which matches what we see around us every day...

Is there any way out of this for large organisations?

Not really sure it's possible. I know some do - but my gut feel tells me this only occurs by chance. If they happen to stumble across a peach by accident, or if one decides to take pity on an organisation and work at making it a better place. But it certainly doesn't seem to be the norm.

The only cases I've really known are where peaches don't know their own worth and stay on, thinking they are worth less than they are. In a mediocre organisation this might last for a while - as their worth is determined by people who don't know how to accurately judge worth. But eventually a true peach will learn about themselves and leave - usually by going outside of their colleague-group and seeing what other people are acheiving in their field - ie by getting a better frame of reference.

Any opinions?

Notes

  • [1]By Tim Harford - pretty good read, actually.
  • [2]for a programmer with my experience - as measured by two different salary surveys. Note: for australia many IT managers use the Hays salary survey which is great... except that it leaves job titles somewhat vague - and several of them don't specifically have a "years experience" associated with them (especially the oft-misunderstood term "junior developer"). When going for your pay review, I recommend searching for a variety of surveys - and make sure they have years on them.
  • [3]...an entirely different story that I won't go into here.
  • [4] Paul Graham wrote an article about Great Hackers which also discusses how it's hard for hackers to judge the abilities of other hackers without having actually worked with them.
  • [5]It is not impossible for a worker to have both sets. It is more rare, however, as a skillset takes time and effort to develop... and your IT workers are generally spending more time developing their IT skills than their social ones.

Wednesday, 12 December 2007

Rails 2.0

Ok, so Rails 2.0 is out - in case you somehow missed the wild whooping sounds from the Rails community.

At the following site is a comprehensive list of changes in Rails 2.0. Worth studying a few times.

Tuesday, 11 December 2007

Rails gotchas: Update attribute... doesn't

Ok, it does... but it doesn't *just* update the attribute - it resaves the whole record. This is a problem if you aren't sure whether or not your record is fresh as you can overwrite other changes with your stale data.

Lesson:

As nasty as it is, you should do a do a self.reload before you do an update_attribute - if it's possible that your record has changed under you (eg by a called sub-method)

Thursday, 6 December 2007

Rails Gotchas: fixtures created_at when?

So - I updated some date-sensitive fixtures and re-ran all my rake tests... and suddenly everything went funny.

First off the rack: empty created_at date

The first problem seemed to be that the created_at date just didn't get set at all. I even printed it out in my test case with an "inspect" and it was showing: "created_at" => "0000-00-00 00:00:00". Pretty nasty.

I checked the fixture and even printed out the date I was trying to save (Time.now.utc) to make sure that Time was working in the fixtures... to no avail, until I realised I'd accidentally knocked off the .to_s(:db) from the end. I added that back and magickally my fixtures were loading the dates again (yay).

Next up: intermittent wierdness

So then there was an intermittant, but persistent error that was really weird. The method I was testing boils down to me pulling out a set of objects and ordering them by creation-date and checking the value of the last one in line. I was checking that if I created a new one - I actually got the new value out the other end. Now the problem was - sometimes I did... and sometimes I didn't... sometimes I'd run rake-test and it'd work... and I'd run it straight afterward and it'd break again.

Then I remembered that datetimes only have a one-second granularity... and some of my tests probably ran less than one second after setup. So I used 1.second.ago.utc.to_s(:db) in the fixtures and it all went away. :P

Friday, 30 November 2007

Pick your layer

I've been noticing that some of our newer Rails developers are having trouble picking which layer (Model, View or Controller) a method belongs in. Here are some heuristics I've picked up along the way - in no particular order.

1 - Don't put it into a library unless it really has nothing to do with the rest of your site (or you want to package it up to hand to other people).

I've recently noticed a few methods being added to a library I wrote that like this: my_method(@my_object) This is a prime candidate for going into the MyObject model itself. Especially when the body of said method just accesses some related objects eg:

# instead of:
module MyLibrary
  def my_method(my_obj)
    my_obj.sub_classes.map {|sc| [sc.name, sc.id] }
  end
end
# put it on the model
class MyModel < ActiveRecord::Base
  has_many :sub_classes
  def my_method
    return [] if sub_classes.blank?
    sub_classes.map {|sc| [sc.name, sc.id] }
  end
end

2 - A model should know how to deal with itself

I'd like to say that everything that can go into a model should... but that's not quite true. Anything data-related, however, is a prime candidate. A model should know how to deal with it's own data. It should know how to calcuate and derive from its own data. It should know how to pull stuff out of related objects, and to parse other objects into a form that allows it to stuff data into itself.

I've seen too many of these sorts of calculations going into views or helpers, or getting stuck in the middle of controller-code. These will only come back to bite you later, when the client suddenly realises they want the report to also be generated into pdf or graphical form as well...

If it can go on the model, put it there, because you'll always have access to the methods on your model.

3 - Don't put html into your model

By all means put display-names and other, similar methods that produce displayable information... but don't put html into it. After all, right now you may be concentrating on web-only delivery of your data... but who's to say the client won't be asking for xml next week... and pdf the week after that... followed by CSV?

Make your model-based display methods text-only eg:

class MyModel < ActiveRecord::Base
  def good_display_name
    name || "Unspecified"
  end
  def bad_display_name
    "<div class=\"left_aligned_red_box\">#{good_display_name}</div>"
  end
end

4 - Put format-specific display functions in the helpers

If you're displaying a selection of a certain class of objects (eg Dates) in a specific format in multiple places across the site, and want consistency of appearance (a Good Thing), feel free to add a display helper to the ApplicationHelper file, or even update the generic display function for that class in your environment.rb

  # eg add these to application helper
  def display_percentage(num)
    return "" unless num
    "#{number_with_precision(num,2)}%"
  end
  def display_date(date)
    return "" unless date
    date.strftime("%a %d/%m/%Y")
  end
  def display_datetime(datetime)
    return "" unless datetime
    "#{display_date(datetime)} #{datetime.strftime("%H:%M")}"
  end

Note that you can over-ride the default datetime display in rails using the following (in environment.rb or similar) then use "to_s" or "to_s(:datetime)" in your views.

ActiveSupport::CoreExtensions::Time::Conversions::DATE_FORMATS.merge!(
   :default => "%a %d/%m/%Y",
   :datetime  => "%a %d/%m/%Y %H:%M"
 )

Monday, 26 November 2007

Rails gotchas: ruby-enhanced yaml != rails-enhanced yaml

I you've just added some funky ruby-generated something into your yaml fixture and are suddenly getting this error message:

Fixture::FormatError: a YAML error occurred parsing /myproject/config/../test/fixtures/widgets.yml. Please note that YAML must be consistently indented using spaces. Tabs are not allowed. Please have a look at http://www.yaml.org/faq.html
The exact error was:
  SyntaxError: compile error
(erb):8: syntax error, unexpected ')'
_erbout.concat "  created_at: "; _erbout.concat(( '1 January 2006'.to_date.to_s(:db) -).to_s); _erbout.concat "\n"
                                                                                       ^

Check to make sure you haven't done the "good" thing and used -%>. It appears that YAML files don't like the whitespace-eating ending... you can only use: %>

Monday, 19 November 2007

Rails gotchas: Kamikaze data migrations

This is a pernicious little gotcha I've encountered a couple of times, generally by using somebody else's plugins. The problem with it is that it usually doesn't actually affect the developer - just anybody else that uses the system. This can lead to the developer refusing to believe that there is a problem.[1]

So what am I talking about here?

I'm talking about a certain type of data migration that explodes as you run it, sometimes even causing the database to get stuck in a nasty halfway-inbetween state where you can neither continue migrating up... or roll back to a previous stable state.

"But migrations are transactional" I hear you cry. Sure they are. That doesn't stop this particular error from occurring... and *still* leaving the db in a nasty stuck state - honest, just gimme a chance to explain why.

Picture this: You've been happily developing away building the next generation of your category-killer Web 2.0 Widget application when you suddenly realise that your widgets need to refer to pieces instead of parts. So, like a good agile developer you:

  • Create a new Piece resource with all the bells and whistles.
  • Create and run the migration that a) creates the pieces table b) data migrates widgets so they point at pieces instead of parts c) drops the parts table.
  • Update all references in your code from pieces to parts.
  • Remove all the part code from controllers/views etc.

All goes smoothly. rake test runs flawlessly and you check in your code. You continue on your merry way, adding more functionality and reflecting on your brilliance...

...until a user of your code emails you to say that your migration breaks and has left their db in some wierd state that doesn't let them migrate up or down.

The problem is that when you (the developer) ran the migration, you still had the Part class in app/models and the Widget class still had references like "has_one :part" in them. When your user downloads your system, however, it may be several changes later... and the system has no recollection of a Part class or how they relate to Widgets. So when you have a data migration such as:

   Widget.find_all.each { |w| make_piece_from_part(w.part) }

It won't know what a part is - or how you get one off your widget class. The nasty part is that often you will find this kind of data migration stuffed in the middle of the schema migrations thus:

def self.up
  create_table :pieces do |t|
     #... creates a table here
  end
  add_column :widgets, :piece_id, :integer, :default => nil
  Widget.find_all.each { |w| make_piece_from_part(w.part) }
  remove_column :widgets, :part_id
end
def self.up
  add_column :widgets, :part_id, :integer, :default => nil
  Widget.find_all.each { |w| make_part_from_piece(w.piece) }
  remove_column :widgets, :piece_id
  drop_table :pieces
end

I'm told that migrations are transactional - ie if it fails, then everything rolls back to the state before the migration... this doesn't seem to work when data migrations are involved. If the data part of the migration falls over, it stops halfway through the migration with a nasty backtrace, but doesn't perform any rollback. So at that point it's already added the pieces table and the piece_id column to the widgets table, but it hasn't removed the part_id column and the schema is still pointing at the migration number before this one. So if you try to run the migration again, it will fail on the first line, saying something like:

== MovePartsToPieces: migrating ============================================
-- create_table(:pieces)
rake aborted!
Mysql::Error: Table 'pieces' already exists: CREATE TABLE pieces (<piece-table insert statement here>) ENGINE=InnoDB

(See full trace by running task with --trace)

Unfortunately it also won't migrate you back. The bad data migration failed before it had a chance to update the current migration level - so migrating back gives you entirely unhelpful "finished in 0.000183" seconds message (or similar), without doing anything at all.

Recovering from the stuck state

Recovering is possible, but it takes some small amount of hacking in the migration. Basically you need to force Rails to run an empty migration so that it does nothing except to put the version number up. Then you can force it to run the bits of the down migration that cancel the bits that it did the first time you tried running the up migration. In detail:

  1. Comment out every line of code in self.up
  2. Comment out the appropriate lines in self.down that correspond with stuff that never got run when the migration fell over the first time - ie leave anything that cancels out actions that were *successfully* done when you did the up migration. in the example above, you'd comment out the downward data migration and the addition of the piece_id column - leaving only the removal of the part_id column and the drop table - which would leave you back in the state you started in.
  3. Save the file
  4. Now you can run rake db:migrate. This should should leave you with a database that at least has the right version number for you to be able to migrate back down again.
  5. Now run the down migration with rake db:migrate VERSION=<one before this one>.
  6. Now go fix your migration so this doesn't happen to anybody else.

Fixing this kind of data-migration nightmare is the only time that I recommend you go back and edit old migrations rather than creating a new one.

  • [1]Ok, so the real reason I'm writing this post is partly so that I can point said unbelievers at it the next time they swear that nothing's wrong and that I should just quit fussing about.

Wednesday, 14 November 2007

Securing Rails

Quick post today. Just found a really great Rails Security Cheatsheet here. Has links to useful articles on all sorts of topics about rails security, including (but not limited to):

  • Why do security for Rails
  • Securing your sessions
  • Useful security plugins
  • Securing files on your server
  • Common web security issues (XSS, SQL injection etc)

Wednesday, 24 October 2007

Keeping passwords out of your logs

By default, rails will insert all parameters into the logs. This is useful for debugging, but not so grand when the parameters are sensitive eg:
Parameters: {"user"=>{"login => "mylogin", "answer"=>"my answer", "question"=>"my question ", "terms"=>"1", "new_password"=>"mypass123", "new_password_confirmation"=>"mypass123"}, commit => "Activate my account", "_method"=>"put"} ...
OR
Parameters: {"commit"=>"Log in", "action"=>"create", "controller"=>"sessions", "password"=>"mypass123", "login"=>"mylogin"} ...

It's a one-liner for rails to filter this out. Just add this line to the top of the relevant controller(s):

class SessionsController < ApplicationController
  # filter out sensitive fields from the log
  filter_parameter_logging 'password'
  # rest of controller goes here...
end
class UsersController < ApplicationController
  # filter out sensitive fields from the log
  filter_parameter_logging 'password', 'question', 'answer', 'given_answer'
  # rest of controller goes here...
end

This will turn the required log lines into:
Parameters: {"user"=>{"login => "mylogin", "answer"=>"[FILTERED]", "question"=>"[FILTERED] ", "terms"=>"1", "new_password"=>"[FILTERED]", "new_password_confirmation"=>"[FILTERED]"}, commit => "Activate my account", "_method"=>"put"} ...
OR
Parameters: {"commit"=>"Log in", "action"=>"create", "controller"=>"sessions", "password"=>"[FILTERED]", "login"=>"mylogin"} ...

Note that it's even smart enough to automatically filter out the "password_confirmation" field without requiring a specific reference to it.

Tuesday, 23 October 2007

Extending ActiveRecord::Validations for positive numbers

I noticed I could DRY up a lot of my def validate items by finding a better way to check if a numerical field was a positive number (when provided). I figured this would be a perfect candidate for extending the "validates_numericality_of" method. But then I had a play with extending it and got stuck on "how exactly do you extend a class method?"

I've successfully added new validations by reopening ActiveRecord::Base, but how do you open an exising method and add more bits on?

I tried alising the method... but the question became: how do you alias a method that is defined as self.validates_whatever?

This excellent post describes how to extend class methods by using base.class_eval and putting the alises into class << self

Note: you definitely need to double-alias your function (as in the class extension section below) or you will find yourself instantly spiralling into a "stack level too deep" exception the first time you call it.

Here's how to extend a validation in Rails (includes my extension to validates_numericality_of, and also my preivous validates_percentage method):

module MyNewValidations
  # magic to allow us to override existing validations
  def self.included(base)
    base.extend(ClassMethods)
    base.class_eval do
      class << self
        alias old_numericality_of :validates_numericality_of unless method_defined?(:old_numericality_of)
        alias validates_numericality_of :my_validates_numericality
      end
    end
  end

  module ClassMethods
    # extends the "validates numericality of" validation to allow the option
    # ":positive_only => true" or ":negative_only => true"
    # This will validate to true only if the given number is positive (>= 0)
    # or negative (<= 0) respectively
    # Otherwise is behaves exactly as the standard validation
    def my_validates_numericality(fields, args = {})
      ret = old_numericality_of(fields, args) # first call standard numericality

      pos = args[:positive_only] || false
      neg = args[:negative_only] || false

      if pos || neg
        msg = args[:message] || "should be a #{pos ? 'positive' : 'negative'} number"
        validates_each fields do |model, attr, val|
          if (pos && val.to_f < 0) || (neg && val.to_f > 0)
            model.errors.add attr, msg
            ret = false
          end
        end
      end

      ret
    end

    # validates whether the given object is a percentage
    # Can also take optional args which will get passed verbatim into the
    # validation methods. Thus it's only really safe to use ":allow_nil" and
    # ":message"
    def validates_percentage(fields, args = {})
      msg = args[:message] || "should be a percentage (0-100)"
      validates_each fields do |model, attr, val|
         pre_val = model.send("#{attr}_before_type_cast".to_sym)
         unless val.nil? || !pre_val.is_a?(String) || pre_val =~ /^[-+]?\d+(\.\d*)?%?$/
           model.errors.add(attr, msg)
         end
       end
      args[:message] = msg
      args[:in] = 0..100
      validates_inclusion_of fields, args
    end

  end
end

class ActiveRecord::Base
  # add in the extra validations created above
  include MyNewValidations

  #other stuff I'd defined goes here
end