Yesterday on Growstuff I noticed that the ‘New garden’ page would let you create a garden with a negative number for the area. That’s not right!

I checked on staging to see if it was already fixed, and then had a look in the Pivotal tracker to see if it had been reported. Nothing showed up, so I set about fixing the problem.

First I made a new branch on my GitHub fork of Growstuff using the web interface, then I switched to it in my local clone…

$ git checkout negative-area

… and opened it in my editor.

I found what I was looking for under app/models in the garden.rb file:

validates :area,
    :numericality => { :only_integer => false },
    :allow_nil => true

That appears to be making sure the value (if present) is numeric. It doesn’t look like it prevents negative numbers, which is the behavior I noticed.

I found the documentation for “numericality” in Active Record Validations and made this change:

validates :area,
-   :numericality => { :only_integer => false },
+   :numericality => { :only_integer => false, :greater_than_or_equal_to => 0 },
    :allow_nil => true

Then I started the app locally, tried it out, and it worked! Or rather, creating a garden with a negative number for the area DIDN’T work, which is what I wanted.

Now, though, the guilt set in. I had changed code without a failing test! The git stash command is useful here, to save a work in progress and pick it up later.

I added a feature test in spec/features/gardens_spec.rb

scenario "Refuse to create new garden with negative area" do
    visit new_garden_path
    fill_in "Name", :with => "Negative Garden"
    fill_in "Area", :with => -5
    click_button "Save"
    expect(page).not_to have_content "Garden was successfully created"
    expect(page).to have_content "Area must be greater than or equal to 0"
  end

That failed as expected, and then passed after I git stash popped my change back into place.

All done! Or so I thought. I submitted my pull request, and immediately got a comment asking for a unit test in addition to the feature test.

That belongs in spec/models/garden_spec.rb:

it "doesn't allow negative area" do
      @garden = FactoryGirl.build(:garden, :area => -5)
      @garden.should_not be_valid
    end

I removed my code change to make sure this test also failed, then restored my patch and tried it again to see it pass. I committed the second test and was about to ask how to update the pull request when I noticed that it happened automatically!

A committer already reviewed and approved it, but the dev branch is frozen while they prepare for a release. It will get merged after that happens, probably some time next week.

All in all that was a lot of typing for a very small bug that wasn’t really hurting anything, but now the app is a tiny bit better, and I know a few more things about Rails and Active Record.