From 7bb7a18b66ee0886af0764956b4469ae14d6c0f7 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Sat, 9 Apr 2016 21:45:13 +0930 Subject: [PATCH 1/7] Fixes #850 Refactors percentage grown to model, stops rendering progress bars in situations it doesn't make sense. Adds specs. --- app/models/planting.rb | 20 +++++++++ .../plantings/_planting_progress.html.haml | 12 ++---- app/views/plantings/_thumbnail.html.haml | 2 +- spec/models/planting_spec.rb | 42 +++++++++++++++++++ 4 files changed, 67 insertions(+), 9 deletions(-) diff --git a/app/models/planting.rb b/app/models/planting.rb index 900156b2a..2e9e22149 100644 --- a/app/models/planting.rb +++ b/app/models/planting.rb @@ -109,6 +109,26 @@ class Planting < ActiveRecord::Base end end + def planted?(current_date = DateTime.now) + planted_at.present? && current_date.to_date >= planted_at + end + + def percentage_grown(current_date = DateTime.now) + return nil unless days_before_maturity && planted?(current_date) + + return 0 if current_date < planted_at + return 100 if days > days_before_maturity + + days = current_date - planted_at + percent = (days/days_before_maturity*100).to_i + + if percent >= 100 + percent = 100 + end + + percent + end + # return a list of interesting plantings, for the homepage etc. # we can't do this via a scope (as far as we know) so sadly we have to # do it this way. diff --git a/app/views/plantings/_planting_progress.html.haml b/app/views/plantings/_planting_progress.html.haml index eea698c82..57cdaeaf6 100644 --- a/app/views/plantings/_planting_progress.html.haml +++ b/app/views/plantings/_planting_progress.html.haml @@ -1,14 +1,10 @@ -- if (planting.planted_at.nil? || DateTime.now.to_date < planting.planted_at) +- unless planting.planted? = "Progress: 0% - not planted yet" - = render partial: "plantings/progress_bar", locals: {status: "warning", progress: "100%"} - elsif planting.finished? = "Progress: 100%" = render partial: "plantings/progress_bar", locals: {status: "success", progress: "100%"} - elsif planting.days_before_maturity.nil? - = "Progress: 0% - Days before maturity unknown" - = render partial: "plantings/progress_bar", locals: {status: "danger", progress: "100%"} + = "Progress: Not calculated, days before maturity unknown" - else - - if (percent = (((DateTime.now - planting.planted_at)/planting.days_before_maturity*100).to_i)) >= 100 - - percent = 100 - = "Progress: #{percent}%" - = render partial: "plantings/progress_bar", locals: {status: "success", progress: "#{percent}%"} \ No newline at end of file + = "Progress: #{planting.percentage_grown}%" + = render partial: "plantings/progress_bar", locals: {status: "success", progress: "#{planting.percentage_grown}%"} \ No newline at end of file diff --git a/app/views/plantings/_thumbnail.html.haml b/app/views/plantings/_thumbnail.html.haml index 9790cb127..fbc859963 100644 --- a/app/views/plantings/_thumbnail.html.haml +++ b/app/views/plantings/_thumbnail.html.haml @@ -38,4 +38,4 @@ %dd= "#{display_days_before_maturity(planting)}" .col-xs-9.col-md-8 - = render partial: 'plantings/planting_progress', locals: {:planting => planting} \ No newline at end of file + = render partial: 'plantings/planting_progress', locals: {planting: planting} \ No newline at end of file diff --git a/spec/models/planting_spec.rb b/spec/models/planting_spec.rb index 659636c5e..1575c9320 100644 --- a/spec/models/planting_spec.rb +++ b/spec/models/planting_spec.rb @@ -37,6 +37,48 @@ describe Planting do Planting.first.should eq @planting2 end + describe '#planted?' do + it "should be false for future plantings" + it "should be false for never planted" + it "should be false for future plantings" + end + + describe '#percentage_grown' do + it 'should not be more than 100%' do + @planting = FactoryGirl.build(:planting, days_before_maturity: 1, planted_at: 1.day.ago) + + now_later_than_planting = 2.days.from_now + + @planting.percentage_grown(now_later_than_planting).should be 100 + end + + it 'should not be less than 0%' do + @planting = FactoryGirl.build(:planting, days_before_maturity: 1, planted_at: 1.day.ago) + + now_earlier_than_planting = 2.days.ago + + @planting.percentage_grown(now_earlier_than_planting).should be 0 + end + + it 'should reflect the current growth' do + @planting = FactoryGirl.build(:planting, days_before_maturity: 10, planted_at: 4.days.ago) + + @planting.percentage_grown.should be 40 + end + + it 'should not be calculated for unplanted plantings' do + @planting = FactoryGirl.build(:planting, planted_at: nil) + + @planting.planted?.should be false + @planting.percentage_grown.should be nil + end + it 'should not be calculated for plantings with an unknown days before maturity' do + @planting = FactoryGirl.build(:planting, days_before_maturity: nil) + + @planting.percentage_grown.should be nil + end + end + context 'delegation' do it 'system name' do planting.crop_name.should eq planting.crop.name From 69fb98146b296bdc6cec5a0070c355a1e167001d Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Sat, 9 Apr 2016 22:01:05 +0930 Subject: [PATCH 2/7] Adjust logic --- app/views/plantings/_planting_progress.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/plantings/_planting_progress.html.haml b/app/views/plantings/_planting_progress.html.haml index 57cdaeaf6..7fe6d52f7 100644 --- a/app/views/plantings/_planting_progress.html.haml +++ b/app/views/plantings/_planting_progress.html.haml @@ -1,4 +1,4 @@ -- unless planting.planted? +- if !planting.planted? = "Progress: 0% - not planted yet" - elsif planting.finished? = "Progress: 100%" From 5cfa051d75e54ddfd3c56021b876d1a85784c369 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Sat, 9 Apr 2016 22:43:17 +0930 Subject: [PATCH 3/7] Update expectations --- spec/features/plantings/planting_a_crop_spec.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/features/plantings/planting_a_crop_spec.rb b/spec/features/plantings/planting_a_crop_spec.rb index 753bbe457..06b4480ea 100644 --- a/spec/features/plantings/planting_a_crop_spec.rb +++ b/spec/features/plantings/planting_a_crop_spec.rb @@ -42,7 +42,7 @@ feature "Planting a crop", :js do end expect(page).to have_content "Planting was successfully created" - expect(page).to have_content "Progress: 0% - Days before maturity unknown" + expect(page).to have_content "Progress: Not calculated, days before maturity unknown" end scenario "Clicking link to owner's profile" do @@ -87,7 +87,7 @@ feature "Planting a crop", :js do end expect(page).to have_content "Planting was successfully created" - expect(page).to have_content "Progress: 0% - Days before maturity unknown" + expect(page).to have_content "Progress: Not calculated, days before maturity unknown" expect(page).to have_content "Days until maturity: unknown" end @@ -106,7 +106,7 @@ feature "Planting a crop", :js do expect(page).to have_content "Planting was successfully created" expect(page).to_not have_content "Progress: 0% - not planted yet" - expect(page).to_not have_content "Progress: 0% - Days before maturity unknown" + expect(page).to_not have_content "Progress: Not calculated, days before maturity unknown" end it "should show that planting is 100% complete (no date specified)" do @@ -169,13 +169,13 @@ feature "Planting a crop", :js do scenario "Editing a planting to fill in the finished date" do visit planting_path(planting) - expect(page).to have_content "Progress: 0% - Days before maturity unknown" + expect(page).to have_content "Progress: Not calculated, days before maturity unknown" click_link "Edit" check "finished" fill_in "Finished date", with: "2015-06-25" click_button "Save" expect(page).to have_content "Planting was successfully updated" - expect(page).to_not have_content "Progress: 0% - Days before maturity unknown" + expect(page).to_not have_content "Progress: Not calculated, days before maturity unknown" end scenario "Marking a planting as finished" do From 3644a8124f4ddfae4383d3dff005a327ff2a3e50 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Sat, 9 Apr 2016 22:54:03 +0930 Subject: [PATCH 4/7] Tweak specs, implementation properly --- app/models/planting.rb | 8 ++++---- spec/models/planting_spec.rb | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/models/planting.rb b/app/models/planting.rb index 2e9e22149..b9566c62b 100644 --- a/app/models/planting.rb +++ b/app/models/planting.rb @@ -109,17 +109,17 @@ class Planting < ActiveRecord::Base end end - def planted?(current_date = DateTime.now) + def planted?(current_date = Date.today) planted_at.present? && current_date.to_date >= planted_at end - def percentage_grown(current_date = DateTime.now) + def percentage_grown(current_date = Date.today) return nil unless days_before_maturity && planted?(current_date) + days = (current_date.to_date - planted_at.to_date).to_i + return 0 if current_date < planted_at return 100 if days > days_before_maturity - - days = current_date - planted_at percent = (days/days_before_maturity*100).to_i if percent >= 100 diff --git a/spec/models/planting_spec.rb b/spec/models/planting_spec.rb index 1575c9320..90b4da713 100644 --- a/spec/models/planting_spec.rb +++ b/spec/models/planting_spec.rb @@ -57,13 +57,13 @@ describe Planting do now_earlier_than_planting = 2.days.ago - @planting.percentage_grown(now_earlier_than_planting).should be 0 + @planting.percentage_grown(now_earlier_than_planting).should be nil end it 'should reflect the current growth' do @planting = FactoryGirl.build(:planting, days_before_maturity: 10, planted_at: 4.days.ago) - @planting.percentage_grown.should be 40 + @planting.percentage_grown(Date.today).should be 40 end it 'should not be calculated for unplanted plantings' do From 82553d6e0a42273918f5681959d1eb070e666475 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Mon, 11 Apr 2016 09:29:06 +0930 Subject: [PATCH 5/7] Avoid time travel in favour of calculating a number of past/future dates --- .../plantings/planting_a_crop_spec.rb | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/spec/features/plantings/planting_a_crop_spec.rb b/spec/features/plantings/planting_a_crop_spec.rb index 06b4480ea..fc255d313 100644 --- a/spec/features/plantings/planting_a_crop_spec.rb +++ b/spec/features/plantings/planting_a_crop_spec.rb @@ -53,16 +53,19 @@ feature "Planting a crop", :js do describe "Progress bar status on planting creation" do before do - DateTime.stub(:now) { DateTime.new(2015, 10, 20, 10, 34) } login_as member visit new_planting_path + + @a_past_date = 15.days.ago.strftime("%Y-%m-%d") + @right_now = Date.today.strftime("%Y-%m-%d") + @a_future_date = 1.years.from_now.strftime("%Y-%m-%d") end it "should show that it is not planted yet" do fill_autocomplete "crop", with: "mai" select_from_autocomplete "maize" within "form#new_planting" do - fill_in "When", with: "2015-12-15" + fill_in "When", with: @a_future_date fill_in "How many?", with: 42 select "cutting", from: "Planted from:" select "semi-shade", from: "Sun or shade?" @@ -78,7 +81,7 @@ feature "Planting a crop", :js do fill_autocomplete "crop", with: "mai" select_from_autocomplete "maize" within "form#new_planting" do - fill_in "When", with: "2015-9-15" + fill_in "When", with: @a_past_date fill_in "How many?", with: 42 select "cutting", from: "Planted from:" select "semi-shade", from: "Sun or shade?" @@ -95,12 +98,12 @@ feature "Planting a crop", :js do fill_autocomplete "crop", with: "mai" select_from_autocomplete "maize" within "form#new_planting" do - fill_in "When", with: "2015-10-15" + fill_in "When", with: @right_now fill_in "How many?", with: 42 select "cutting", from: "Planted from:" select "semi-shade", from: "Sun or shade?" fill_in "Tell us more about it", with: "It's rad." - fill_in "Finished date", with: "2015-10-30" + fill_in "Finished date", with: @a_future_date click_button "Save" end @@ -113,7 +116,7 @@ feature "Planting a crop", :js do fill_autocomplete "crop", with: "mai" select_from_autocomplete "maize" within "form#new_planting" do - fill_in "When", with: "2015-10-15" + fill_in "When", with: @right_now fill_in "How many?", with: 42 select "cutting", from: "Planted from:" select "semi-shade", from: "Sun or shade?" @@ -132,12 +135,12 @@ feature "Planting a crop", :js do fill_autocomplete "crop", with: "mai" select_from_autocomplete "maize" within "form#new_planting" do - fill_in "When", with: "2015-10-15" + fill_in "When", with: @a_past_date fill_in "How many?", with: 42 select "cutting", from: "Planted from:" select "semi-shade", from: "Sun or shade?" fill_in "Tell us more about it", with: "It's rad." - fill_in "Finished date", with: "2015-10-19" + fill_in "Finished date", with: @right_now click_button "Save" end From 12ad16a05a5bf6ba1ed9af003d2ea74693f772d5 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Wed, 13 Apr 2016 08:41:05 +0930 Subject: [PATCH 6/7] Newlines --- spec/models/planting_spec.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/spec/models/planting_spec.rb b/spec/models/planting_spec.rb index 90b4da713..dfccd8352 100644 --- a/spec/models/planting_spec.rb +++ b/spec/models/planting_spec.rb @@ -72,6 +72,7 @@ describe Planting do @planting.planted?.should be false @planting.percentage_grown.should be nil end + it 'should not be calculated for plantings with an unknown days before maturity' do @planting = FactoryGirl.build(:planting, days_before_maturity: nil) @@ -83,12 +84,15 @@ describe Planting do it 'system name' do planting.crop_name.should eq planting.crop.name end + it 'wikipedia url' do planting.crop_en_wikipedia_url.should eq planting.crop.en_wikipedia_url end + it 'default scientific name' do planting.crop_default_scientific_name.should eq planting.crop.default_scientific_name end + it 'plantings count' do planting.crop_plantings_count.should eq planting.crop.plantings_count end @@ -309,13 +313,11 @@ describe Planting do @f = FactoryGirl.build(:planting, :planted_at => '2013-01-01', :finished_at => nil) @f.should be_valid end + it 'allows just the finished date' do @f = FactoryGirl.build(:planting, :finished_at => '2013-01-01', :planted_at => nil) @f.should be_valid end - end - end - -end +end \ No newline at end of file From cf0a6466996e3dd9ee04d37cf3f56f869228cecd Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Thu, 14 Apr 2016 09:59:36 +0930 Subject: [PATCH 7/7] Add EOF newlines --- app/views/plantings/_planting_progress.html.haml | 2 +- app/views/plantings/_thumbnail.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/plantings/_planting_progress.html.haml b/app/views/plantings/_planting_progress.html.haml index 7fe6d52f7..a9c851709 100644 --- a/app/views/plantings/_planting_progress.html.haml +++ b/app/views/plantings/_planting_progress.html.haml @@ -7,4 +7,4 @@ = "Progress: Not calculated, days before maturity unknown" - else = "Progress: #{planting.percentage_grown}%" - = render partial: "plantings/progress_bar", locals: {status: "success", progress: "#{planting.percentage_grown}%"} \ No newline at end of file + = render partial: "plantings/progress_bar", locals: {status: "success", progress: "#{planting.percentage_grown}%"} diff --git a/app/views/plantings/_thumbnail.html.haml b/app/views/plantings/_thumbnail.html.haml index fbc859963..df62f7a9c 100644 --- a/app/views/plantings/_thumbnail.html.haml +++ b/app/views/plantings/_thumbnail.html.haml @@ -38,4 +38,4 @@ %dd= "#{display_days_before_maturity(planting)}" .col-xs-9.col-md-8 - = render partial: 'plantings/planting_progress', locals: {planting: planting} \ No newline at end of file + = render partial: 'plantings/planting_progress', locals: {planting: planting}