Merge pull request #699 from tygriffin/fix-request-new-crop

Fix request new crop

app/controllers/crops_controller.rb

@@ -105,6 +105,10 @@ class CropsController < ApplicationController
   # GET /crops/1/edit
   def edit
     @crop = Crop.find(params[:id])
+
+    (3 - @crop.scientific_names.length).times do
+      @crop.scientific_names.build
+    end
   end
 
   # POST /crops
@@ -123,7 +127,7 @@ class CropsController < ApplicationController
 
     respond_to do |format|
       if @crop.save
-        if current_member.has_role? :crop_wrangler
+        unless current_member.has_role? :crop_wrangler
           Role.crop_wranglers.each do |w|
             Notifier.new_crop_request(w, @crop).deliver!
           end
@@ -179,6 +183,6 @@ class CropsController < ApplicationController
   private
 
   def crop_params
-    params.require(:crop).permit(:en_wikipedia_url, :name, :parent_id, :creator_id, :approval_status, :request_notes, :reason_for_rejection, :scientific_names_attributes => [:scientific_name, :_destroy, :id])
+    params.require(:crop).permit(:en_wikipedia_url, :name, :parent_id, :creator_id, :approval_status, :request_notes, :reason_for_rejection, :rejection_notes, :scientific_names_attributes => [:scientific_name, :_destroy, :id])
   end
 end

app/mailers/notifier.rb

@@ -24,7 +24,7 @@ class Notifier < ActionMailer::Base
 
   def new_crop_request(member, request)
     @member, @request = member, request
-    mail(:to => @member.email, :subject => "New crop request")    
+    mail(:to => @member.email, :subject => "#{@request.requester.login_name} has requested #{@request.name} as a new crop")    
   end
 
   def crop_request_approved(member, crop)

app/models/ability.rb

@@ -25,6 +25,16 @@ class Ability
     # are wranglers or admins 
     cannot :read, Crop
     can :read, Crop, :approval_status => "approved"
+    # scientific names should only be viewable if associated crop is approved
+    cannot :read, ScientificName
+    can :read, ScientificName do |sn|
+      sn.crop.approved?
+    end
+    # ... same for alternate names
+    cannot :read, AlternateName
+    can :read, AlternateName do |an|
+      an.crop.approved?
+    end
 
     if member
       # members can see even rejected or pending crops if they requested it

app/models/crop.rb

@@ -43,6 +43,10 @@ class Crop < ActiveRecord::Base
   ## This validation addresses a race condition
   validate :approval_status_cannot_be_changed_again
 
+  validate :must_be_rejected_if_rejected_reasons_present
+
+  validate :must_have_meaningful_reason_for_rejection
+
   ####################################
   # Elastic search configuration
   include Elasticsearch::Model
@@ -311,7 +315,7 @@ class Crop < ActiveRecord::Base
       )
       return response.records.to_a
     else
-      where("name ILIKE ?", "%#{query}%") 
+      where("name ILIKE ?", "%#{query}%").load
     end
   end
 
@@ -324,4 +328,18 @@ class Crop < ActiveRecord::Base
     end
   end
 
+  def must_be_rejected_if_rejected_reasons_present
+    unless rejected?
+      if reason_for_rejection.present? || rejection_notes.present?
+        errors.add(:approval_status, "must be rejected if a reason for rejection is present")
+      end
+    end
+  end
+
+  def must_have_meaningful_reason_for_rejection
+    if reason_for_rejection == "other" && rejection_notes.blank?
+      errors.add(:rejection_notes, "must be added if the reason for rejection is \"other\"")
+    end
+  end
+
 end

app/models/harvest.rb

@@ -20,6 +20,8 @@ class Harvest < ActiveRecord::Base
 
   default_scope { order('created_at DESC') }
 
+  validates :crop, :approved => true
+
   validates :crop, :presence => {:message => "must be present and exist in our database"}
 
   validates :quantity,

app/models/planting.rb

@@ -30,7 +30,9 @@ class Planting < ActiveRecord::Base
 
   default_scope { order("created_at desc") }
 
-  validates :crop_id, :presence => {:message => "must be present and exist in our database"}
+  validates :crop, :approved => true
+
+  validates :crop, :presence => {:message => "must be present and exist in our database"}
 
   validates :quantity,
     :numericality => {

app/models/seed.rb

@@ -7,6 +7,8 @@ class Seed < ActiveRecord::Base
 
   default_scope { order("created_at desc") }
 
+  validates :crop, :approved => true
+
   validates :crop, :presence => {:message => "must be present and exist in our database"}
   validates :quantity,
     :numericality => {

app/validators/approved_validator.rb

@@ -0,0 +1,7 @@
+class ApprovedValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    unless record.crop.try(:approved?)
+      record.errors[attribute] << (options[:message] || 'must be approved')
+    end
+  end
+end

app/views/crops/_form.html.haml

@@ -66,6 +66,13 @@
         = f.label :reason_for_rejection, 'Reason for rejection', :class => 'control-label col-md-2'
         .col-md-8
           = f.select(:reason_for_rejection, @crop.reasons_for_rejection, {:include_blank => true}, {:class => 'form-control'})
+      %p
+        %span.help-block
+          Please provide additional notes why this crop request was rejected if the above reasons do not apply.
+
+      .form-group
+        = f.label :rejection_notes, 'Rejection Notes', :class => 'control-label col-md-2'
+        .col-md-8= f.text_area :rejection_notes, :rows => 6, :class => 'form-control'
 
   - else
     %p

app/views/crops/show.html.haml

@@ -2,26 +2,27 @@
 - content_for :subtitle, @crop.default_scientific_name
 
 - if @crop.pending?
-  .alert.alert-warning
+  .alert.alert-danger
     %b This crop is currently pending approval.
-    %p This crop was requested by #{@crop.requester} on #{@crop.created_at}.
+    %p This crop was requested by #{@crop.requester} #{time_ago_in_words(@crop.created_at)} ago.
     - unless @crop.request_notes.blank?
       %p
         Request notes: #{@crop.request_notes}
 
 - if @crop.rejected?
-  .alert.alert-warning
-    %b This crop was rejected for the following reason: #{@crop.reason_for_rejection}.
+  .alert.alert-danger
+    %b This crop was rejected for the following reason: #{@crop.reason_for_rejection == "other" ? @crop.rejection_notes : @crop.reason_for_rejection}.
 
-- content_for :buttonbar do
-  - if can? :create, Planting
-    = link_to "Plant this", new_planting_path(:crop_id => @crop.id), :class => 'btn btn-default'
+- if @crop.approved?
+  - content_for :buttonbar do
+    - if can? :create, Planting
+      = link_to "Plant this", new_planting_path(:crop_id => @crop.id), :class => 'btn btn-default'
 
-  - if can? :create, Harvest
-    = link_to "Harvest this", new_harvest_path(:crop_id => @crop.id), :class => 'btn btn-default'
+    - if can? :create, Harvest
+      = link_to "Harvest this", new_harvest_path(:crop_id => @crop.id), :class => 'btn btn-default'
 
-  - if can? :create, Seed
-    = link_to 'Add seeds to stash', new_seed_path(:params => { :crop_id => @crop.id }), :class => 'btn btn-default'
+    - if can? :create, Seed
+      = link_to 'Add seeds to stash', new_seed_path(:params => { :crop_id => @crop.id }), :class => 'btn btn-default'
 
 
 .row

app/views/crops/wrangle.html.haml

@@ -18,7 +18,7 @@
 .tabbable
   %ul.nav.nav-tabs
     %li{:class => @approval_status.blank? ? 'active' : ''}
-      = link_to "Recendly added", wrangle_crops_path
+      = link_to "Recently added", wrangle_crops_path
     %li{:class => @approval_status == "pending" ? 'active' : ''}
       = link_to "Pending approval", wrangle_crops_path(:approval_status => "pending")
     %li{:class => @approval_status == "rejected" ? 'active' : ''}
@@ -42,13 +42,11 @@
     %th System name
     %th English Wikipedia URL
     %th Scientific names
-    %th 
-      - if @approval_status == "pending"
-        Requested by
-      - elsif @approval_status == "rejected"
-        Rejected by
-      - else
-        Added by
+    %th Requested by
+    - if @approval_status == "rejected"
+      %th Rejected by
+    - if @approval_status != "rejected" && @approval_status != "pending"
+      %th Added by
     %th When
   - @crops.each do |c|
     %tr
@@ -58,11 +56,9 @@
         - c.scientific_names.each do |s|
           = link_to s.scientific_name, s
           %br/
-      %td
-        - if @approval_status == "pending"
-          = link_to c.requester, c.requester
-        - else
-          = link_to c.creator, c.creator
+      %td= c.requester.present? ? (link_to c.requester, c.requester) : "N/A"
+      - unless @approval_status == "pending"
+        %td= c.creator.present? ? (link_to c.creator, c.creator) : "N/A"
       %td
         = distance_of_time_in_words(c.created_at, Time.zone.now)
         ago.

app/views/notifier/_signature.html.haml

@@ -0,0 +1,6 @@
+- site_name = ENV['GROWSTUFF_SITE_NAME']
+
+%p
+  The #{site_name} team.
+  %br/
+  =link_to root_url, root_url

app/views/notifier/crop_request_approved.html.haml

@@ -11,3 +11,6 @@
     = link_to "Harvest #{@crop.name}", new_harvest_url(crop_id: @crop.id)
   %li
     = link_to "Stash seeds for #{@crop.name}", new_seed_url(crop_id: @crop.id)
+
+= render :partial => 'signature'
+

app/views/notifier/crop_request_rejected.html.haml

@@ -4,4 +4,6 @@
 %p
   Your request for the new crop: #{link_to @crop.name, crop_url(@crop)} has been rejected for the following reason.
 
-  = @crop.reason_for_rejection
+  = @crop.reason_for_rejection
+
+= render :partial => 'signature'

app/views/notifier/new_crop_request.html.haml

@@ -1,7 +1,21 @@
 - site_name = ENV['GROWSTUFF_SITE_NAME']
+
 %p Hello #{@member.login_name},
 
-%p
-  A new crop has been requested. View, edit, approve or reject the request:
+%p 
+  #{@request.requester.login_name} has requested a new crop on #{site_name}.
 
-  = link_to "Request for #{@request}", crop_url(@request)
+%ul
+  %li Name: #{@request.name}
+  %li Wikipedia URL: #{@request.en_wikipedia_url.present? ? @request.en_wikipedia_url : "not specified"}
+
+%p
+  As a crop wrangler, you can #{link_to "approve or reject this request", edit_crop_url(@request)}.
+
+%p
+  Or, discuss this and other crop wrangling issues in our #{link_to "crop wrangling forum", "http://talk.growstuff.org/c/crop-wrangling"}.
+
+%p
+  Thanks for your help!
+
+= render :partial => 'signature'

app/views/notifier/notify.html.haml

@@ -20,7 +20,4 @@
   %br/
   = link_to "Turn off these notifications", edit_member_registration_url
 
-%p
-  The #{site_name} team.
-  %br/
-  =link_to root_url, root_url
+= render :partial => 'signature'

app/views/notifier/planting_reminder.html.haml

@@ -59,10 +59,7 @@
 %h2
   See you soon on #{site_name}!
 
-%p
-  The #{site_name} team.
-  %br/
-  =link_to root_url, root_url
+= render :partial => 'signature'
 
 %hr/
 %p

db/migrate/20150209105410_add_rejection_notes_to_crops.rb

@@ -0,0 +1,5 @@
+class AddRejectionNotesToCrops < ActiveRecord::Migration
+  def change
+    add_column :crops, :rejection_notes, :text
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20150201064502) do
+ActiveRecord::Schema.define(version: 20150209105410) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -74,6 +74,7 @@ ActiveRecord::Schema.define(version: 20150201064502) do
     t.string   "approval_status",      default: "approved"
     t.text     "reason_for_rejection"
     t.text     "request_notes"
+    t.text     "rejection_notes"
   end
 
   add_index "crops", ["name"], name: "index_crops_on_name", using: :btree
@@ -285,6 +286,7 @@ ActiveRecord::Schema.define(version: 20150201064502) do
     t.datetime "updated_at"
     t.string   "slug"
     t.integer  "forum_id"
+    t.integer  "parent_id"
   end
 
   add_index "posts", ["created_at", "author_id"], name: "index_posts_on_created_at_and_author_id", using: :btree

spec/features/crops/request_new_crop_spec.rb

@@ -5,6 +5,7 @@ feature "Requesting a new crop" do
   context "As a regular member" do
 
     let(:member) { FactoryGirl.create(:member) }
+    let!(:wrangler) { FactoryGirl.create(:crop_wrangling_member) }
 
     before { login_as member }
 

spec/mailers/notifier_spec.rb

@@ -52,7 +52,7 @@ describe Notifier do
     let(:mail) { Notifier.new_crop_request(member, crop) }
 
     it 'sets the subject correctly' do
-      mail.subject.should == "New crop request"
+      mail.subject.should == "#{crop.requester.login_name} has requested Ultra berry as a new crop"
     end
 
     it 'comes from noreply@growstuff.org' do