From fbc64a1ee2b2549a7b3482bc133991242e849942 Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Thu, 11 Jul 2013 12:13:38 +0100 Subject: [PATCH 1/6] Crop: method to return hash of sunniness values. --- app/models/crop.rb | 10 ++++++++++ spec/factories/planting.rb | 12 ++++++++++++ spec/models/crop_spec.rb | 29 +++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/app/models/crop.rb b/app/models/crop.rb index acb04d6d5..913ee09c2 100644 --- a/app/models/crop.rb +++ b/app/models/crop.rb @@ -43,4 +43,14 @@ class Crop < ActiveRecord::Base return photos.first end + def sunniness + sunniness = Hash.new(0) + plantings.each do |p| + if p.sunniness + sunniness[p.sunniness] += 1 + end + end + return sunniness + end + end diff --git a/spec/factories/planting.rb b/spec/factories/planting.rb index 963b7a2b0..fde1eaf59 100644 --- a/spec/factories/planting.rb +++ b/spec/factories/planting.rb @@ -7,5 +7,17 @@ FactoryGirl.define do description "This is a *really* good plant." sunniness 'sun' planted_from 'seed' + + factory :sunny_planting do + sunniness 'sun' + end + + factory :semi_shady_planting do + sunniness 'semi-shade' + end + + factory :shady_planting do + sunniness 'shade' + end end end diff --git a/spec/models/crop_spec.rb b/spec/models/crop_spec.rb index 3184d236e..190403f3b 100644 --- a/spec/models/crop_spec.rb +++ b/spec/models/crop_spec.rb @@ -79,4 +79,33 @@ describe Crop do @crop.default_photo.should be_an_instance_of Photo end end + + context 'sunniness' do + before(:each) do + @crop = FactoryGirl.create(:tomato) + end + + it 'returns a hash of sunniness values' do + planting1 = FactoryGirl.create(:sunny_planting, :crop => @crop) + planting2 = FactoryGirl.create(:sunny_planting, :crop => @crop) + planting3 = FactoryGirl.create(:semi_shady_planting, :crop => @crop) + planting4 = FactoryGirl.create(:shady_planting, :crop => @crop) + @crop.sunniness.should be_an_instance_of Hash + end + + it 'counts each sunniness value' do + planting1 = FactoryGirl.create(:sunny_planting, :crop => @crop) + planting2 = FactoryGirl.create(:sunny_planting, :crop => @crop) + planting3 = FactoryGirl.create(:semi_shady_planting, :crop => @crop) + planting4 = FactoryGirl.create(:shady_planting, :crop => @crop) + @crop.sunniness.should == { 'sun' => 2, 'shade' => 1, 'semi-shade' => 1 } + end + + it 'ignores unused sunniness values' do + planting1 = FactoryGirl.create(:sunny_planting, :crop => @crop) + planting2 = FactoryGirl.create(:sunny_planting, :crop => @crop) + planting3 = FactoryGirl.create(:semi_shady_planting, :crop => @crop) + @crop.sunniness.should == { 'sun' => 2, 'semi-shade' => 1 } + end + end end From 744b526dd1cee3f39105831efe8fef6ff88b06a1 Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Thu, 11 Jul 2013 12:41:22 +0100 Subject: [PATCH 2/6] Show sunniness frequencies on crop page Also cleaned up tests that were relying on the default planting factory to have a sunniness value set. --- app/views/crops/show.html.haml | 5 ++ spec/factories/planting.rb | 1 - spec/models/planting_spec.rb | 4 ++ spec/views/crops/show.html.haml_spec.rb | 62 ++++++++++++++------- spec/views/plantings/show.html.haml_spec.rb | 5 ++ 5 files changed, 57 insertions(+), 20 deletions(-) diff --git a/app/views/crops/show.html.haml b/app/views/crops/show.html.haml index f9a08a076..31b242492 100644 --- a/app/views/crops/show.html.haml +++ b/app/views/crops/show.html.haml @@ -14,6 +14,11 @@ = succeed ":" do = @crop.system_name != @crop.varieties.map{ |c| link_to c, c }.join(", ") + - if @crop.sunniness.length > 0 + %p + Plant in: + - sunniness = @crop.sunniness.sort_by {|s, freq| freq }.reverse + = sunniness.map {|s, freq| "#{s} (#{freq})" }.join(", ") - if @crop.plantings_count > 0 %p diff --git a/spec/factories/planting.rb b/spec/factories/planting.rb index fde1eaf59..b6948927b 100644 --- a/spec/factories/planting.rb +++ b/spec/factories/planting.rb @@ -5,7 +5,6 @@ FactoryGirl.define do planted_at Date.today quantity 33 description "This is a *really* good plant." - sunniness 'sun' planted_from 'seed' factory :sunny_planting do diff --git a/spec/models/planting_spec.rb b/spec/models/planting_spec.rb index ca2d4e02d..a565bdd2f 100644 --- a/spec/models/planting_spec.rb +++ b/spec/models/planting_spec.rb @@ -50,6 +50,10 @@ describe Planting do end context 'sunniness' do + before(:each) do + @planting = FactoryGirl.create(:sunny_planting) + end + it 'should have a sunniness value' do @planting.sunniness.should eq 'sun' end diff --git a/spec/views/crops/show.html.haml_spec.rb b/spec/views/crops/show.html.haml_spec.rb index b1bfe26da..967fe3e97 100644 --- a/spec/views/crops/show.html.haml_spec.rb +++ b/spec/views/crops/show.html.haml_spec.rb @@ -6,15 +6,7 @@ describe "crops/show" do @crop = FactoryGirl.create(:maize, :scientific_names => [ FactoryGirl.create(:zea_mays) ] ) - @owner = FactoryGirl.create(:member) - @garden = FactoryGirl.create(:garden, :owner => @owner) - @planting = FactoryGirl.create(:planting, - :garden => @garden, - :crop => @crop - ) - assign(:crop, @crop) - end it "shows the wikipedia URL" do @@ -38,18 +30,50 @@ describe "crops/show" do assert_select("a[href=#{new_planting_path}?crop_id=#{@crop.id}]") end - it "links to people who are growing this crop" do - render - rendered.should contain /member\d+/ - rendered.should contain "Springfield Community Garden" - end + context "has plantings" do + before(:each) do + @owner = FactoryGirl.create(:member) + @garden = FactoryGirl.create(:garden, :owner => @owner) + @planting = FactoryGirl.create(:planting, + :garden => @garden, + :crop => @crop + ) + end + + it "doesn't show sunniness if none are set" do + render + rendered.should_not contain "Plant in:" + end + + it "shows sunniness frequencies" do + FactoryGirl.create(:sunny_planting, :crop => @crop) + render + rendered.should contain "Plant in:" + rendered.should contain "sun (1)" + end + + it "shows multiple sunniness frequencies" do + FactoryGirl.create(:sunny_planting, :crop => @crop) + FactoryGirl.create(:sunny_planting, :crop => @crop) + FactoryGirl.create(:shady_planting, :crop => @crop) + render + rendered.should contain "Plant in:" + rendered.should contain "sun (2), shade (1)" + end + + it "links to people who are growing this crop" do + render + rendered.should contain /member\d+/ + rendered.should contain "Springfield Community Garden" + end + + it "shows photos where available" do + @photo = FactoryGirl.create(:photo) + @planting.photos << @photo + render + assert_select "img", :src => @photo.thumbnail_url + end - it "shows photos where available" do - @planting = FactoryGirl.create(:planting, :crop => @crop) - @photo = FactoryGirl.create(:photo) - @planting.photos << @photo - render - assert_select "img", :src => @photo.thumbnail_url end context 'varieties' do diff --git a/spec/views/plantings/show.html.haml_spec.rb b/spec/views/plantings/show.html.haml_spec.rb index ddd8db657..2cef7b131 100644 --- a/spec/views/plantings/show.html.haml_spec.rb +++ b/spec/views/plantings/show.html.haml_spec.rb @@ -16,6 +16,11 @@ describe "plantings/show" do end context 'sunniness' do + before(:each) do + @p = assign(:planting, + FactoryGirl.create(:sunny_planting) + ) + end it "shows the sunniness" do render From 4d9ddd27076dcff806309b6f28bd057f0358787e Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Thu, 11 Jul 2013 13:01:52 +0100 Subject: [PATCH 3/6] Crop: method to return hash of planted_from frequencies Also tidied up tests which relied on the default planting factory having a planted_from value set. --- app/models/crop.rb | 10 +++++++ spec/factories/planting.rb | 13 ++++++++- spec/models/crop_spec.rb | 31 +++++++++++++++++++++ spec/models/planting_spec.rb | 1 + spec/views/plantings/show.html.haml_spec.rb | 4 +++ 5 files changed, 58 insertions(+), 1 deletion(-) diff --git a/app/models/crop.rb b/app/models/crop.rb index 913ee09c2..3574315f5 100644 --- a/app/models/crop.rb +++ b/app/models/crop.rb @@ -53,4 +53,14 @@ class Crop < ActiveRecord::Base return sunniness end + def planted_from + planted_from = Hash.new(0) + plantings.each do |p| + if p.planted_from + planted_from[p.planted_from] += 1 + end + end + return planted_from + end + end diff --git a/spec/factories/planting.rb b/spec/factories/planting.rb index b6948927b..58f67f653 100644 --- a/spec/factories/planting.rb +++ b/spec/factories/planting.rb @@ -5,7 +5,18 @@ FactoryGirl.define do planted_at Date.today quantity 33 description "This is a *really* good plant." - planted_from 'seed' + + factory :seed_planting do + planted_from 'seed' + end + + factory :seedling_planting do + planted_from 'seedling' + end + + factory :cutting_planting do + planted_from 'cutting' + end factory :sunny_planting do sunniness 'sun' diff --git a/spec/models/crop_spec.rb b/spec/models/crop_spec.rb index 190403f3b..6c66d2399 100644 --- a/spec/models/crop_spec.rb +++ b/spec/models/crop_spec.rb @@ -108,4 +108,35 @@ describe Crop do @crop.sunniness.should == { 'sun' => 2, 'semi-shade' => 1 } end end + + context 'planted_from' do + before(:each) do + @crop = FactoryGirl.create(:tomato) + end + + it 'returns a hash of sunniness values' do + planting1 = FactoryGirl.create(:seed_planting, :crop => @crop) + planting2 = FactoryGirl.create(:seed_planting, :crop => @crop) + planting3 = FactoryGirl.create(:seedling_planting, :crop => @crop) + planting4 = FactoryGirl.create(:cutting_planting, :crop => @crop) + @crop.planted_from.should be_an_instance_of Hash + end + + it 'counts each planted_from value' do + planting1 = FactoryGirl.create(:seed_planting, :crop => @crop) + planting2 = FactoryGirl.create(:seed_planting, :crop => @crop) + planting3 = FactoryGirl.create(:seedling_planting, :crop => @crop) + planting4 = FactoryGirl.create(:cutting_planting, :crop => @crop) + @crop.planted_from.should == { 'seed' => 2, 'seedling' => 1, 'cutting' => 1 } + end + + it 'ignores unused planted_from values' do + planting1 = FactoryGirl.create(:seed_planting, :crop => @crop) + planting2 = FactoryGirl.create(:seed_planting, :crop => @crop) + planting3 = FactoryGirl.create(:seedling_planting, :crop => @crop) + @crop.planted_from.should == { 'seed' => 2, 'seedling' => 1 } + end + end + + end diff --git a/spec/models/planting_spec.rb b/spec/models/planting_spec.rb index a565bdd2f..692c53284 100644 --- a/spec/models/planting_spec.rb +++ b/spec/models/planting_spec.rb @@ -74,6 +74,7 @@ describe Planting do context 'planted from' do it 'should have a planted_from value' do + @planting = FactoryGirl.create(:seed_planting) @planting.planted_from.should eq 'seed' end diff --git a/spec/views/plantings/show.html.haml_spec.rb b/spec/views/plantings/show.html.haml_spec.rb index 2cef7b131..3cc735a86 100644 --- a/spec/views/plantings/show.html.haml_spec.rb +++ b/spec/views/plantings/show.html.haml_spec.rb @@ -38,6 +38,10 @@ describe "plantings/show" do end context 'planted from' do + before(:each) do + @p = assign(:planting, FactoryGirl.create(:seed_planting)) + end + it "shows planted_from" do render rendered.should contain 'Planted from:' From c4f4f4f7efa78743a64b191a0d3e6dc79f800be4 Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Thu, 11 Jul 2013 13:08:01 +0100 Subject: [PATCH 4/6] Show planted_from frequencies on crop page. --- app/views/crops/show.html.haml | 5 +++++ spec/views/crops/show.html.haml_spec.rb | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/app/views/crops/show.html.haml b/app/views/crops/show.html.haml index 31b242492..5183041b1 100644 --- a/app/views/crops/show.html.haml +++ b/app/views/crops/show.html.haml @@ -14,6 +14,11 @@ = succeed ":" do = @crop.system_name != @crop.varieties.map{ |c| link_to c, c }.join(", ") + - if @crop.planted_from.length > 0 + %p + Plant from: + - planted_from = @crop.planted_from.sort_by {|s, freq| freq }.reverse + = planted_from.map {|s, freq| "#{s} (#{freq})" }.join(", ") - if @crop.sunniness.length > 0 %p Plant in: diff --git a/spec/views/crops/show.html.haml_spec.rb b/spec/views/crops/show.html.haml_spec.rb index 967fe3e97..f59d77b9a 100644 --- a/spec/views/crops/show.html.haml_spec.rb +++ b/spec/views/crops/show.html.haml_spec.rb @@ -61,6 +61,27 @@ describe "crops/show" do rendered.should contain "sun (2), shade (1)" end + it "doesn't show planted_from if none are set" do + render + rendered.should_not contain "Plant from:" + end + + it "shows planted_from frequencies" do + FactoryGirl.create(:seed_planting, :crop => @crop) + render + rendered.should contain "Plant from:" + rendered.should contain "seed (1)" + end + + it "shows multiple planted_from frequencies" do + FactoryGirl.create(:seed_planting, :crop => @crop) + FactoryGirl.create(:seed_planting, :crop => @crop) + FactoryGirl.create(:cutting_planting, :crop => @crop) + render + rendered.should contain "Plant from:" + rendered.should contain "seed (2), cutting (1)" + end + it "links to people who are growing this crop" do render rendered.should contain /member\d+/ From ece34149a29621cf65fc07d7e2855dca0d9ce1c2 Mon Sep 17 00:00:00 2001 From: Skud Date: Thu, 11 Jul 2013 22:15:56 +1000 Subject: [PATCH 5/6] need to test sunniness/planted_from.blank? --- app/models/crop.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/crop.rb b/app/models/crop.rb index 3574315f5..06b504274 100644 --- a/app/models/crop.rb +++ b/app/models/crop.rb @@ -46,7 +46,7 @@ class Crop < ActiveRecord::Base def sunniness sunniness = Hash.new(0) plantings.each do |p| - if p.sunniness + if !p.sunniness.blank? sunniness[p.sunniness] += 1 end end @@ -56,7 +56,7 @@ class Crop < ActiveRecord::Base def planted_from planted_from = Hash.new(0) plantings.each do |p| - if p.planted_from + if !p.planted_from.blank? planted_from[p.planted_from] += 1 end end From 1867f8b07f480c248330055e94c82809e8867205 Mon Sep 17 00:00:00 2001 From: Skud Date: Tue, 23 Jul 2013 19:33:02 +1000 Subject: [PATCH 6/6] minor wording changes --- CONTRIBUTING.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b43c0d134..156c83eba 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,7 +2,7 @@ Thanks for contributing to Growstuff! We have different contribution guidelines depending on whether your change is a small one (a one-line bugfix or similar) or a larger one. -## Small changes (no more than a couple of lines) +## Small changes Send us a pull request! We will get one of our pairs of coders to review it. @@ -14,8 +14,9 @@ wiki](http://wiki.growstuff.org/) for more information. ## Larger changes Growstuff does pair programming (two coders working together) for all -significant changes. This means that if you submit a pull request and -weren't working in a pair, we can't merge it into the project as-is. +new features and other significant changes. This means that if you +submit a pull request and weren't working in a pair, we're unlikely to +merge it into the project as-is. If you would like to work on any larger change, we would appreciate it if you would get in touch with us, preferably via our [mailing @@ -26,9 +27,9 @@ The [Growstuff wiki](http://wiki.growstuff.org/) has lots more information on our dev process, to get you started if you would like to join us. -If you submit a significant change without working in a pair, we will -treat your work as an experimental "spike" and get one of our pairs of +If you submit a larger change without working in a pair, we will treat +your work as an experimental "spike" and get one of our pairs of programmers to look over it and maybe use what you've done as the basis -for re-implementing it using our processes. **We'd much rather work with -you, so please talk to us first!** +for re-implementing it using our processes. **We'd much rather work +with you, so please talk to us first!**