Revert "Revert "Merge remote-tracking branch 'henare_github/patch-1'""

This reverts commit 211fe84dc40d97df8aa8724906d9170ed4f78477.

19 files changed, 156 insertions, 86 deletions

diff --git a/.rvmrc b/.rvmrc
index 28471f674..1f2700fc2 100644
--- a/.rvmrc
+++ b/.rvmrc
@@ -29,7 +29,6 @@ then
     \. "${rvm_path:-$HOME/.rvm}/hooks/after_use" || true
   if [[ $- == *i* ]] # check for interactive shells
   then echo "Using: $(tput setaf 2)$GEM_HOME$(tput sgr0)" # show the user the ruby and gemset they are using in green
-  else echo "Using: $GEM_HOME" # don't use colors in non-interactive shells
   fi
 else
   # If the environment file has not yet been created, use the RVM CLI to select.
diff --git a/app/controllers/admin_request_controller.rb b/app/controllers/admin_request_controller.rb
index 7cf23e61e..53ff2957b 100644
--- a/app/controllers/admin_request_controller.rb
+++ b/app/controllers/admin_request_controller.rb
@@ -71,6 +71,7 @@ class AdminRequestController < AdminController
         old_allow_new_responses_from = @info_request.allow_new_responses_from
         old_handle_rejected_responses = @info_request.handle_rejected_responses
         old_tag_string = @info_request.tag_string
+        old_comments_allowed = @info_request.comments_allowed
 
         @info_request.title = params[:info_request][:title]
         @info_request.prominence = params[:info_request][:prominence]
@@ -81,6 +82,7 @@ class AdminRequestController < AdminController
         @info_request.allow_new_responses_from = params[:info_request][:allow_new_responses_from]
         @info_request.handle_rejected_responses = params[:info_request][:handle_rejected_responses]
         @info_request.tag_string = params[:info_request][:tag_string]
+        @info_request.comments_allowed = params[:info_request][:comments_allowed] == "true" ? true : false
 
         if @info_request.valid?
             @info_request.save!
@@ -92,7 +94,8 @@ class AdminRequestController < AdminController
                     :old_awaiting_description => old_awaiting_description, :awaiting_description => @info_request.awaiting_description,
                     :old_allow_new_responses_from => old_allow_new_responses_from, :allow_new_responses_from => @info_request.allow_new_responses_from,
                     :old_handle_rejected_responses => old_handle_rejected_responses, :handle_rejected_responses => @info_request.handle_rejected_responses,
-                    :old_tag_string => old_tag_string, :tag_string => @info_request.tag_string
+                    :old_tag_string => old_tag_string, :tag_string => @info_request.tag_string,
+                    :old_comments_allowed => old_comments_allowed, :tag_string => @info_request.comments_allowed
                 })
             # expire cached files
             expire_for_request(@info_request)
diff --git a/app/controllers/comment_controller.rb b/app/controllers/comment_controller.rb
index 1552017c2..988a8a6f8 100644
--- a/app/controllers/comment_controller.rb
+++ b/app/controllers/comment_controller.rb
@@ -23,6 +23,13 @@ class CommentController < ApplicationController
         else
             raise "Unknown type " + params[:type]
         end
+        
+        # Are comments disabled on this request?
+        #
+        # There is no “add comment” link when comments are disabled, so users should
+        # not usually hit this unless they are explicitly attempting to avoid the comment
+        # block, so we just raise an exception.
+        raise "Comments are not allowed on this request" if !@info_request.comments_allowed?
 
         # Banned from adding comments?
         if !authenticated_user.nil? && !authenticated_user.can_make_comments?
diff --git a/app/models/censor_rule.rb b/app/models/censor_rule.rb
index da3f49760..a74fdcb24 100644
--- a/app/models/censor_rule.rb
+++ b/app/models/censor_rule.rb
@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 114
+# Schema version: 20120919140404
 #
 # Table name: censor_rules
 #
@@ -9,11 +9,11 @@
 #  public_body_id    :integer
 #  text              :text            not null
 #  replacement       :text            not null
-#  regexp            :boolean
 #  last_edit_editor  :string(255)     not null
 #  last_edit_comment :text            not null
 #  created_at        :datetime        not null
 #  updated_at        :datetime        not null
+#  regexp            :boolean
 #
 
 # models/censor_rule.rb:
diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index 2e16d0f58..b62f67ee1 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -1,4 +1,5 @@
 # == Schema Information
+# Schema version: 20120919140404
 #
 # Table name: info_requests
 #
@@ -19,6 +20,7 @@
 #  external_user_name        :string(255)
 #  external_url              :string(255)
 #  attention_requested       :boolean         default(FALSE)
+#  comments_allowed          :boolean         default(TRUE), not null
 #
 
 require 'digest/sha1'
diff --git a/app/models/public_body.rb b/app/models/public_body.rb
index 77da81d4c..2cf1ce8a2 100644
--- a/app/models/public_body.rb
+++ b/app/models/public_body.rb
@@ -1,32 +1,27 @@
-# -*- coding: utf-8 -*-
 # == Schema Information
+# Schema version: 20120919140404
 #
 # Table name: public_bodies
 #
-#  id                 :integer         not null, primary key
-#  name               :text            not null
-#  short_name         :text            not null
-#  request_email      :text            not null
-#  version            :integer         not null
-#  last_edit_editor   :string(255)     not null
-#  last_edit_comment  :text            not null
-#  created_at         :datetime        not null
-#  updated_at         :datetime        not null
-#  url_name           :text            not null
-#  home_page          :text            default(""), not null
-#  notes              :text            default(""), not null
-#  first_letter       :string(255)     not null
-#  publication_scheme :text            default(""), not null
-#  api_key            :string(255)     not null
-#
-# models/public_body.rb:
-# A public body, from which information can be requested.
+#  id                  :integer         not null, primary key
+#  name                :text            not null
+#  short_name          :text            not null
+#  request_email       :text            not null
+#  version             :integer         not null
+#  last_edit_editor    :string(255)     not null
+#  last_edit_comment   :text            not null
+#  created_at          :datetime        not null
+#  updated_at          :datetime        not null
+#  url_name            :text            not null
+#  home_page           :text            default(""), not null
+#  notes               :text            default(""), not null
+#  first_letter        :string(255)     not null
+#  publication_scheme  :text            default(""), not null
+#  api_key             :string(255)
+#  info_requests_count :integer         default(0), not null
 #
-# Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
-# Email: francis@mysociety.org; WWW: http://www.mysociety.org/
-#
-# $Id: public_body.rb,v 1.160 2009-10-02 22:56:35 francis Exp $
 
+# -*- coding: utf-8 -*-
 require 'csv'
 require 'securerandom'
 require 'set'
diff --git a/app/models/request_classification.rb b/app/models/request_classification.rb
index 678b6cd16..f5a1b4bee 100644
--- a/app/models/request_classification.rb
+++ b/app/models/request_classification.rb
@@ -1,3 +1,15 @@
+# == Schema Information
+# Schema version: 20120919140404
+#
+# Table name: request_classifications
+#
+#  id                    :integer         not null, primary key
+#  user_id               :integer
+#  info_request_event_id :integer
+#  created_at            :datetime
+#  updated_at            :datetime
+#
+
 class RequestClassification < ActiveRecord::Base
     belongs_to :user
 
@@ -13,4 +25,4 @@ class RequestClassification < ActiveRecord::Base
                                          :include => :user)
     end
 
-end
\ No newline at end of file
+end
diff --git a/app/models/user.rb b/app/models/user.rb
index 657ea2a4a..bb1b54d70 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1,4 +1,5 @@
 # == Schema Information
+# Schema version: 20120919140404
 #
 # Table name: users
 #
@@ -21,13 +22,6 @@
 #  no_limit               :boolean         default(FALSE), not null
 #  receive_email_alerts   :boolean         default(TRUE), not null
 #
-# models/user.rb:
-# Model of people who use the site to file requests, make comments etc.
-#
-# Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
-# Email: francis@mysociety.org; WWW: http://www.mysociety.org/
-#
-# $Id: user.rb,v 1.106 2009-10-01 01:43:36 francis Exp $
 
 require 'digest/sha1'
 
diff --git a/app/views/admin_request/edit.rhtml b/app/views/admin_request/edit.rhtml
index 808028b47..8fa2a1fe2 100644
--- a/app/views/admin_request/edit.rhtml
+++ b/app/views/admin_request/edit.rhtml
@@ -28,6 +28,10 @@
    <br/>(don't forget to change 'awaiting description' when you set described state)<br/>
    </p>
 
+   <p><label for="info_request_comments_allowed"><strong>Are comments allowed?</strong></label>
+   <%= select('info_request', "comments_allowed", [["Yes – comments allowed", true], ["No – comments disabled", false]]) %>
+   </p>
+
    <p><label for="info_request_tag_string"><strong>Tags</strong> <small>(space separated, can use key:value)</small></label><br/>
    <%= text_field 'info_request', 'tag_string', :size => 60  %></p>
 
diff --git a/app/views/request/_after_actions.rhtml b/app/views/request/_after_actions.rhtml
index 15ca6302e..580ff0e87 100644
--- a/app/views/request/_after_actions.rhtml
+++ b/app/views/request/_after_actions.rhtml
@@ -5,9 +5,11 @@
     <div id="anyone_actions">
         <strong><%= _('Anyone:') %></strong>
         <ul>
-        <li>
-      <%= _('<a href="%s">Add an annotation</a> (to help the requester or others)') % [new_comment_url(:url_title => @info_request.url_title)] %>
-        </li>
+        <% if @info_request.comments_allowed? %>
+          <li>
+            <%= _('<a href="%s">Add an annotation</a> (to help the requester or others)') % [new_comment_url(:url_title => @info_request.url_title)] %>
+          </li>
+        <% end %>
         <% if @old_unclassified %>
             <li>
                 <%= link_to _('Update the status of this request'), '#describe_state_form_1' %>
diff --git a/app/views/request/_sidebar.rhtml b/app/views/request/_sidebar.rhtml
index 731bfb34e..dc0d2eb31 100644
--- a/app/views/request/_sidebar.rhtml
+++ b/app/views/request/_sidebar.rhtml
@@ -24,7 +24,7 @@
         <% end %>
      <% else %>
       <p><%= _('Requests for personal information and vexatious requests are not considered valid for FOI purposes (<a href="/help/about">read more</a>).') %></p>
-       <p><%= ('If you believe this request is not suitable, you can report it for attention by the site administrators') %></p>
+       <p><%= _('If you believe this request is not suitable, you can report it for attention by the site administrators') %></p>
        <%= link_to _("Report this request"), report_path, :class => "link_button_green", :method => "POST" %>
       <% end %>
     <% end %>
@@ -32,11 +32,11 @@
 
     <div class="act_link">
      <% tweet_link = "https://twitter.com/share?url=#{h(request.url)}&via=#{h(MySociety::Config.get('TWITTER_USERNAME', ''))}&text='#{h(@info_request.title)}'&related=#{_('alaveteli_foi:The software that runs {{site_name}}', :site_name => h(site_name))}" %>
-        <%= link_to '<img src="/images/twitter-16.png" alt="twitter icon">', tweet_link %> 
-        <%= link_to _("Tweet this request"), tweet_link %> 
+        <%= link_to '<img src="/images/twitter-16.png" alt="twitter icon">', tweet_link %>
+        <%= link_to _("Tweet this request"), tweet_link %>
     </div>
     <div class="act_link">
-        <%= link_to '<img src="/images/wordpress.png" alt="" class="rss">', "http://wordpress.com/"%> 
+        <%= link_to '<img src="/images/wordpress.png" alt="" class="rss">', "http://wordpress.com/"%>
         <%= link_to _("Start your own blog"), "http://wordpress.com/"%>
     </div>
 
diff --git a/app/views/track/_tracking_links.rhtml b/app/views/track/_tracking_links.rhtml
index 39f346eff..3ba9d15e2 100644
--- a/app/views/track/_tracking_links.rhtml
+++ b/app/views/track/_tracking_links.rhtml
@@ -4,12 +4,12 @@
     end
 %>
 
-<% if own_request %> 
+<% if own_request %>
     <p><%= _('This is your own request, so you will be automatically emailed when new responses arrive.')%></p>
-<% elsif existing_track %> 
+<% elsif existing_track %>
     <p><%= track_thing.params[:verb_on_page_already] %></p>
     <div class="feed_link feed_link_<%=location%>">
-	<%= link_to "Unsubscribe", {:controller => 'track', :action => 'update', :track_id => existing_track.id, :track_medium => "delete", :r => request.request_uri}, :class => "link_button_green" %>
+  <%= link_to _("Unsubscribe"), {:controller => 'track', :action => 'update', :track_id => existing_track.id, :track_medium => "delete", :r => request.request_uri}, :class => "link_button_green" %>
     </div>
 <% elsif track_thing %>
     <div class="feed_link feed_link_<%=location%>">
@@ -19,9 +19,9 @@
         <%= link_to _("Follow"), do_track_url(track_thing), :class => "link_button_green" %>
      <% end %>
     </div>
-    
+
     <div class="feed_link feed_link_<%=location%>">
-        <%= link_to '<img src="/images/feed-16.png" alt="">', do_track_url(track_thing, 'feed') %> 
+        <%= link_to '<img src="/images/feed-16.png" alt="">', do_track_url(track_thing, 'feed') %>
         <%= link_to (location == 'sidebar' ? _('RSS feed of updates') : _('RSS feed')),  do_track_url(track_thing, 'feed') %>
     </div>
 <% end %>
diff --git a/config/deploy.rb b/config/deploy.rb
index 888710f83..e0507be2c 100644
--- a/config/deploy.rb
+++ b/config/deploy.rb
@@ -48,6 +48,7 @@ namespace :deploy do
     links = {
       "#{release_path}/config/database.yml" => "#{shared_path}/database.yml",
       "#{release_path}/config/general.yml" => "#{shared_path}/general.yml",
+      "#{release_path}/config/rails_env.rb" => "#{shared_path}/rails_env.rb",
       "#{release_path}/files" => "#{shared_path}/files",
       "#{release_path}/cache" => "#{shared_path}/cache",
       "#{release_path}/vendor/plugins/acts_as_xapian/xapiandbs" => "#{shared_path}/xapiandbs",
diff --git a/db/migrate/20120919140404_add_comments_allowed_to_info_request.rb b/db/migrate/20120919140404_add_comments_allowed_to_info_request.rb
new file mode 100644
index 000000000..4ae0bce18
--- /dev/null
+++ b/db/migrate/20120919140404_add_comments_allowed_to_info_request.rb
@@ -0,0 +1,9 @@
+class AddCommentsAllowedToInfoRequest < ActiveRecord::Migration
+  def self.up
+    add_column :info_requests, :comments_allowed, :boolean, :null => false, :default => true
+  end
+
+  def self.down
+    remove_column :info_requests, :comments_allowed
+  end
+end
diff --git a/doc/INSTALL.md b/doc/INSTALL.md
index 62c5071ca..da72987af 100644
--- a/doc/INSTALL.md
+++ b/doc/INSTALL.md
@@ -39,7 +39,7 @@ If you are running Debian, add the following repositories to
     deb http://ftp.debian.org/debian/ testing main non-free contrib
 
 The repositories above allow us to install the packages
-`wkthmltopdf-static` and `bundler` using `apt`; so if you're running
+`wkhtmltopdf-static` and `bundler` using `apt`; so if you're running
 Ubuntu, you won't be able to use the above repositories, and you will
 need to comment out those two lines in `config/packages` before
 following the next step (and install bundler manually).
@@ -326,12 +326,17 @@ which lives in `config/alert-tracks-debian.ugly`.  As with the cron
 jobs above, replace the variables (and/or bits near the variables)
 with paths to your software.  `config/purge-varnish-debian.ugly` is a
 similar init script, which is optional and not required if you choose
-not to run your site behind Varnish (see below).
+not to run your site behind Varnish (see below). Either tweak the file permissions to make the script executable by your deploy user, or add the following line to your
+sudoers file to allow these to be run by your deploy user (named `deploy` in
+this case):
+
+    deploy  ALL = NOPASSWD: /etc/init.d/foi-alert-tracks, /etc/init.d/foi-purge-varnish
 
 The cron jobs refer to a program `run-with-lockfile`. See
 [this issue](https://github.com/mysociety/alaveteli/issues/112) for a
 discussion of where to find this program, and how you might replace
-it.
+it. This [one line script](https://gist.github.com/3741194) can install
+this program system-wide.
 
 # Set up production web server
 
diff --git a/spec/controllers/comment_controller_spec.rb b/spec/controllers/comment_controller_spec.rb
index b71bc0aea..4a7acee23 100644
--- a/spec/controllers/comment_controller_spec.rb
+++ b/spec/controllers/comment_controller_spec.rb
@@ -53,6 +53,17 @@ describe CommentController, "when commenting on a request" do
 
         response.should render_template('new')
     end
+    
+    it "should not allow comments if comments are not allowed" do
+      session[:user_id] = users(:silly_name_user).id
+
+      expect {
+        post :new, :url_title => info_requests(:spam_1_request).url_title,
+              :comment => { :body => "I demand to be heard!" },
+              :type => 'request', :submitted_comment => 1, :preview => 0
+      }.to raise_error("Comments are not allowed on this request")
+      
+    end
 
     describe 'when commenting on an external request' do
 
diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb
index 95737a250..77f43b618 100644
--- a/spec/controllers/request_controller_spec.rb
+++ b/spec/controllers/request_controller_spec.rb
@@ -238,6 +238,22 @@ describe RequestController, "when showing one request" do
         response.should have_tag('div#owner_actions')
     end
 
+    describe 'when the request does allow comments' do
+      it 'should have a comment link' do
+        get :show, { :url_title => 'why_do_you_have_such_a_fancy_dog' },
+                   { :user_id => users(:admin_user).id }
+        response.should have_tag('#anyone_actions', /Add an annotation/)
+      end
+    end
+    
+    describe 'when the request does not allow comments' do
+      it 'should not have a comment link' do
+        get :show, { :url_title => 'spam_1' },
+                   { :user_id => users(:admin_user).id }
+        response.should_not have_tag('#anyone_actions', /Add an annotation/)
+      end
+    end
+    
     describe 'when the request is being viewed by an admin' do
 
         describe 'if the request is awaiting description' do
diff --git a/spec/fixtures/info_requests.yml b/spec/fixtures/info_requests.yml
index 079a44cd0..9361ec486 100644
--- a/spec/fixtures/info_requests.yml
+++ b/spec/fixtures/info_requests.yml
@@ -8,6 +8,7 @@ fancy_dog_request:
     user_id: 1
     described_state: waiting_response
     awaiting_description: true
+    comments_allowed: true
     idhash: 50929748
 naughty_chicken_request:
     id: 103
@@ -19,6 +20,7 @@ naughty_chicken_request:
     user_id: 1
     described_state: waiting_response
     awaiting_description: false
+    comments_allowed: true
     idhash: e8d18c84
 badger_request:
     id: 104
@@ -30,6 +32,7 @@ badger_request:
     user_id: 1
     described_state: waiting_response
     awaiting_description: false
+    comments_allowed: true
     idhash: e8d18c84
 boring_request:
     id: 105
@@ -41,6 +44,7 @@ boring_request:
     user_id: 1
     described_state: successful
     awaiting_description: false
+    comments_allowed: true
     idhash: 173fd003
 another_boring_request:
     id: 106
@@ -52,6 +56,7 @@ another_boring_request:
     user_id: 1
     described_state: successful
     awaiting_description: false
+    comments_allowed: true
     idhash: 173fd004
 
 # A pair of identical requests (with url_title differing only in the numeric suffix)
@@ -66,6 +71,7 @@ spam_1_request:
     user_id: 5
     described_state: successful
     awaiting_description: false
+    comments_allowed: false
     idhash: 173fd005
 spam_2_request:
     id: 108
@@ -77,6 +83,7 @@ spam_2_request:
     user_id: 5
     described_state: successful
     awaiting_description: false
+    comments_allowed: true
     idhash: 173fd005
 external_request:
     id: 109
@@ -87,6 +94,7 @@ external_request:
     public_body_id: 2
     described_state: waiting_response
     awaiting_description: false
+    comments_allowed: true
     idhash: a1234567
 anonymous_external_request:
     id: 110
@@ -97,4 +105,5 @@ anonymous_external_request:
     public_body_id: 2
     described_state: waiting_response
     awaiting_description: false
-    idhash: 7654321a
\ No newline at end of file
+    comments_allowed: true
+    idhash: 7654321a
diff --git a/spec/views/request/_after_actions.rhtml_spec.rb b/spec/views/request/_after_actions.rhtml_spec.rb
index d04db3fc2..5b4734c52 100644
--- a/spec/views/request/_after_actions.rhtml_spec.rb
+++ b/spec/views/request/_after_actions.rhtml_spec.rb
@@ -1,85 +1,86 @@
 require File.expand_path(File.join('..', '..', '..', 'spec_helper'), __FILE__)
 
-describe 'when displaying actions that can be taken with regard to a request' do 
-    
-    before do 
-        @mock_body = mock_model(PublicBody, :name => 'test public body', 
+describe 'when displaying actions that can be taken with regard to a request' do
+
+    before do
+        @mock_body = mock_model(PublicBody, :name => 'test public body',
                                             :url_name => 'test_public_body')
-        @mock_user = mock_model(User, :name => 'test user', 
+        @mock_user = mock_model(User, :name => 'test user',
                                       :url_name => 'test_user')
-        @mock_request = mock_model(InfoRequest, :title => 'test request', 
-                                                :user => @mock_user, 
-                                                :user_name => @mock_user.name, 
+        @mock_request = mock_model(InfoRequest, :title => 'test request',
+                                                :user => @mock_user,
+                                                :user_name => @mock_user.name,
                                                 :is_external? => false,
-                                                :public_body => @mock_body, 
+                                                :public_body => @mock_body,
+                                                :comments_allowed? => true,
                                                 :url_title => 'test_request')
         assigns[:info_request] = @mock_request
     end
-  
+
     def do_render
         render :partial => 'request/after_actions'
     end
-    
+
     def expect_owner_div
         do_render
         response.should have_tag('div#owner_actions'){ yield }
     end
-    
+
     def expect_anyone_div
         do_render
         response.should have_tag('div#anyone_actions'){ yield }
     end
-    
+
     def expect_owner_link(text)
         expect_owner_div{ with_tag('a', :text => text) }
     end
-    
+
     def expect_no_owner_link(text)
         expect_owner_div{ without_tag('a', :text => text) }
     end
-    
+
     def expect_anyone_link(text)
         expect_anyone_div{ with_tag('a', :text => text) }
     end
-    
+
     def expect_no_anyone_link(text)
         expect_anyone_div{ without_tag('a', :text => text) }
     end
-    
-    describe 'if the request is old and unclassified' do 
-    
-        before do 
+
+    describe 'if the request is old and unclassified' do
+
+        before do
             assigns[:old_unclassified] = true
         end
-        
-        it 'should not display a link for the request owner to update the status of the request' do 
+
+        it 'should not display a link for the request owner to update the status of the request' do
             expect_no_owner_link('Update the status of this request')
         end
-        
-        it 'should display a link for anyone to update the status of the request' do 
+
+        it 'should display a link for anyone to update the status of the request' do
             expect_anyone_link('Update the status of this request')
         end
-        
+
     end
-    
-    describe 'if the request is not old and unclassified' do 
-        
-        before do 
+
+    describe 'if the request is not old and unclassified' do
+
+        before do
             assigns[:old_unclassified] = false
         end
-        
-        it 'should display a link for the request owner to update the status of the request' do 
+
+        it 'should display a link for the request owner to update the status of the request' do
             expect_owner_link('Update the status of this request')
         end
-        
-        it 'should not display a link for anyone to update the status of the request' do 
+
+        it 'should not display a link for anyone to update the status of the request' do
             expect_no_anyone_link('Update the status of this request')
         end
-        
+
     end
 
     it 'should display a link for the request owner to request a review' do
         expect_owner_link('Request an internal review')
     end
-        
+
 end