From 2ecbd8315dd4452952a610cbe01341e3703dd381 Mon Sep 17 00:00:00 2001 From: Skud Date: Tue, 11 Jun 2013 16:07:09 +1000 Subject: [PATCH] bugfix: don't say 'not yet set' in planting form PT: https://www.pivotaltracker.com/story/show/51457917 Maco found this. The problem was that if you had a blank planting date, and then re-edited the planting, it would say "not yet set" in the form field, then die when it later tried to convert that to a date. I replaced Miles's planted_at_string stuff in the model with a simpler parse_date method in the application helper. --- app/controllers/application_controller.rb | 2 ++ app/controllers/plantings_controller.rb | 6 ++++-- app/helpers/application_helper.rb | 5 +++++ app/models/planting.rb | 14 +------------- app/views/plantings/_form.html.haml | 2 +- config/initializers/time_formats.rb | 2 ++ spec/helpers/application_helper.rb | 7 +++++++ spec/models/planting_spec.rb | 20 -------------------- spec/views/plantings/_form.html.haml_spec.rb | 2 +- 9 files changed, 23 insertions(+), 37 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3b4dac042..60a80a80c 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,6 +1,8 @@ class ApplicationController < ActionController::Base protect_from_forgery + include ApplicationHelper + # tweak CanCan defaults because we don't have a "current_user" method # this means that we use current_user in specs but current_member everywhere # else in the code. diff --git a/app/controllers/plantings_controller.rb b/app/controllers/plantings_controller.rb index b8775d1e2..9d4b3f0b9 100644 --- a/app/controllers/plantings_controller.rb +++ b/app/controllers/plantings_controller.rb @@ -30,8 +30,7 @@ class PlantingsController < ApplicationController # GET /plantings/new # GET /plantings/new.json def new - @planting = Planting.new - @planting.planted_at = Date.today + @planting = Planting.new('planted_at' => Date.today) # using find_by_id here because it returns nil, unlike find @crop = Crop.find_by_id(params[:crop_id]) || Crop.new @@ -46,6 +45,7 @@ class PlantingsController < ApplicationController # GET /plantings/1/edit def edit @planting = Planting.find(params[:id]) + # the following are needed to display the form but aren't used @crop = Crop.new @garden = Garden.new @@ -54,6 +54,7 @@ class PlantingsController < ApplicationController # POST /plantings # POST /plantings.json def create + params[:planted_at] = parse_date(params[:planted_at]) @planting = Planting.new(params[:planting]) respond_to do |format| @@ -71,6 +72,7 @@ class PlantingsController < ApplicationController # PUT /plantings/1.json def update @planting = Planting.find(params[:id]) + params[:planted_at] = parse_date(params[:planted_at]) respond_to do |format| if @planting.update_attributes(params[:planting]) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 51da9b2b3..b07e64dd2 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -10,5 +10,10 @@ module ApplicationHelper Growstuff::Application.config.currency) end + def parse_date(str) + str ||= '' # Date.parse barfs on nil + return str == '' ? nil : Date.parse(str) + end + end diff --git a/app/models/planting.rb b/app/models/planting.rb index 08fe34d90..8bf6dda2c 100644 --- a/app/models/planting.rb +++ b/app/models/planting.rb @@ -3,7 +3,7 @@ class Planting < ActiveRecord::Base friendly_id :planting_slug, use: :slugged attr_accessible :crop_id, :description, :garden_id, :planted_at, - :quantity, :sunniness, :planted_at_string + :quantity, :sunniness belongs_to :garden belongs_to :crop @@ -37,18 +37,6 @@ class Planting < ActiveRecord::Base return "#{garden.owner.login_name}'s #{garden}" end - def planted_at_string - if planted_at - planted_at.strftime("%F") - else - "Not yet set" - end - end - - def planted_at_string=(str) - self.planted_at = str == '' ? nil : Time.parse(str) - end - def to_s self.crop_system_name + " in " + self.location end diff --git a/app/views/plantings/_form.html.haml b/app/views/plantings/_form.html.haml index 2a8906add..b21add0c9 100644 --- a/app/views/plantings/_form.html.haml +++ b/app/views/plantings/_form.html.haml @@ -15,7 +15,7 @@ Garden.where(:owner_id => current_member), :id, :name, :selected => @planting.garden_id || @garden.id) .control-group = f.label 'When?', :class => 'control-label' - .controls= f.text_field :planted_at_string, :class => 'add-datepicker' + .controls= f.text_field :planted_at, :value => @planting.planted_at ? @planting.planted_at.to_s(:ymd) : '', :class => 'add-datepicker' .control-group = f.label 'How many?', :class => 'control-label' .controls diff --git a/config/initializers/time_formats.rb b/config/initializers/time_formats.rb index 0b26541d9..3f3adf454 100644 --- a/config/initializers/time_formats.rb +++ b/config/initializers/time_formats.rb @@ -4,4 +4,6 @@ Date::DATE_FORMATS[:default] = "%B %d, %Y" Time::DATE_FORMATS[:date] = "%B %d, %Y" Date::DATE_FORMATS[:date] = "%B %d, %Y" +Date::DATE_FORMATS[:ymd] = "%Y-%m-%d" + Time::DATE_FORMATS[:datetime] = '%B %d, %Y at %H:%M' diff --git a/spec/helpers/application_helper.rb b/spec/helpers/application_helper.rb index c65d0a8aa..912ffbfd5 100644 --- a/spec/helpers/application_helper.rb +++ b/spec/helpers/application_helper.rb @@ -5,4 +5,11 @@ describe ApplicationHelper do price_in_dollars(999).should eq '9.99' price_with_currency(999).should eq '9.99 AUD' end + + it "parses dates" do + parse_date(nil).should eq nil + parse_date('').should eq nil + parse_date('2012-05-12').should eq Date.new(2012, 5, 12) + parse_date('may 12th 2012').should eq Date.new(2012, 5, 12) + end end diff --git a/spec/models/planting_spec.rb b/spec/models/planting_spec.rb index 02cc5a062..cfadcf9cd 100644 --- a/spec/models/planting_spec.rb +++ b/spec/models/planting_spec.rb @@ -29,26 +29,6 @@ describe Planting do @planting.slug.should match /^member\d+-springfield-community-garden-tomato$/ end - it "should accept ISO-format dates" do - @planting.planted_at_string = "2013-03-01" - @planting.planted_at.should == Time.local(2013, 03, 01) - end - - it "should accept DD Month YY format dates" do - @planting.planted_at_string = "1st March 13" # Dydd Gŵyl Dewi Hapus! - @planting.planted_at.should == Time.local(2013, 03, 01) - end - - it 'should accept blank dates' do - @planting.planted_at_string = '' - @planting.planted_at.should == nil - end - - it "should output dates in ISO format" do - @planting.planted_at = Time.local(2013, 03, 01) - @planting.planted_at_string.should == "2013-03-01" - end - it 'should sort in reverse creation order' do @planting2 = FactoryGirl.create(:planting) Planting.first.should eq @planting2 diff --git a/spec/views/plantings/_form.html.haml_spec.rb b/spec/views/plantings/_form.html.haml_spec.rb index 2b6813ada..225b08256 100644 --- a/spec/views/plantings/_form.html.haml_spec.rb +++ b/spec/views/plantings/_form.html.haml_spec.rb @@ -18,7 +18,7 @@ describe "plantings/_form" do end it "has a free-form text field containing the planting date in ISO format" do - assert_select 'input#planting_planted_at_string[type=text][value=2013-03-01]' + assert_select 'input#planting_planted_at[type=text][value=2013-03-01]' end context "logged in" do