diff options
37 files changed, 547 insertions, 133 deletions
diff --git a/Capfile b/Capfile new file mode 100644 index 000000000..6a798eb2b --- /dev/null +++ b/Capfile @@ -0,0 +1,4 @@ +load 'deploy' +# Uncomment if you are using Rails' asset pipeline + # load 'deploy/assets' +load 'config/deploy' # remove this line to skip loading any of the default tasks
\ No newline at end of file @@ -1,5 +1,5 @@ # Work around bug in Debian Squeeze - see https://github.com/mysociety/alaveteli/pull/297#issuecomment-4101012 -if File.exist? "/etc/debian_version" and File.open("/etc/debian_version").read.strip =~ /^(squeeze|6\.0\.[45])$/ +if File.exist? "/etc/debian_version" and File.open("/etc/debian_version").read.strip =~ /^(squeeze.*|6\.0\.[45])$/ if File.exist? "/lib/libuuid.so.1" require 'dl' DL::dlopen('/lib/libuuid.so.1') @@ -11,6 +11,7 @@ gem 'rails', '2.3.14' gem 'pg' gem 'fast_gettext', '>= 0.6.0' +gem 'fastercsv', '>=1.5.5' gem 'gettext_i18n_rails', '>= 0.6.0', :git => "git://github.com/sebbacon/gettext_i18n_rails.git" gem 'gettext', '>= 1.9.3' gem 'json', '~> 1.5.1' @@ -35,6 +36,7 @@ gem 'will_paginate', '~> 2.3.11' gem 'xapian-full-alaveteli', '~> 1.2.9.5' gem 'xml-simple' gem 'zip' +gem 'capistrano' group :test do gem 'fakeweb' diff --git a/Gemfile.lock b/Gemfile.lock index 0d9d5cc1d..841e92112 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -19,11 +19,19 @@ GEM activesupport (= 2.3.14) activesupport (2.3.14) annotate (2.4.0) + capistrano (2.13.3) + highline + net-scp (>= 1.0.0) + net-sftp (>= 2.0.0) + net-ssh (>= 2.0.14) + net-ssh-gateway (>= 1.1.0) columnize (0.3.6) fakeweb (1.3.0) fast_gettext (0.6.8) + fastercsv (1.5.5) gettext (2.2.1) locale + highline (1.6.13) json (1.5.4) linecache (0.46) rbx-require-relative (> 0.0.4) @@ -32,6 +40,13 @@ GEM memcache-client (1.8.5) net-http-local (0.1.2) net-purge (0.1.0) + net-scp (1.0.4) + net-ssh (>= 1.99.1) + net-sftp (2.0.5) + net-ssh (>= 2.0.9) + net-ssh (2.5.2) + net-ssh-gateway (1.1.0) + net-ssh (>= 1.99.1) pg (0.13.2) rack (1.1.3) rails (2.3.14) @@ -72,8 +87,10 @@ PLATFORMS DEPENDENCIES annotate + capistrano fakeweb fast_gettext (>= 0.6.0) + fastercsv (>= 1.5.5) gettext (>= 1.9.3) gettext_i18n_rails (>= 0.6.0)! json (~> 1.5.1) diff --git a/app/controllers/admin_request_controller.rb b/app/controllers/admin_request_controller.rb index c5abf8769..7cf23e61e 100644 --- a/app/controllers/admin_request_controller.rb +++ b/app/controllers/admin_request_controller.rb @@ -16,12 +16,25 @@ class AdminRequestController < AdminController def list @query = params[:query] - @info_requests = InfoRequest.paginate :order => "created_at desc", :page => params[:page], :per_page => 100, + @info_requests = InfoRequest.paginate :order => "created_at desc", + :page => params[:page], + :per_page => 100, :conditions => @query.nil? ? nil : ["lower(title) like lower('%'||?||'%')", @query] end def list_old_unclassified - @info_requests = InfoRequest.find_old_unclassified(:conditions => ["prominence = 'normal'"]) + @info_requests = WillPaginate::Collection.create((params[:page] or 1), 50) do |pager| + info_requests = InfoRequest.find_old_unclassified(:conditions => ["prominence = 'normal'"], + :limit => pager.per_page, + :offset => pager.offset) + # inject the result array into the paginated collection: + pager.replace(info_requests) + + unless pager.total_entries + # the pager didn't manage to guess the total count, do it manually + pager.total_entries = InfoRequest.count_old_unclassified(:conditions => ["prominence = 'normal'"]) + end + end end def show diff --git a/app/controllers/public_body_controller.rb b/app/controllers/public_body_controller.rb index 95d936e54..b8ea82a66 100644 --- a/app/controllers/public_body_controller.rb +++ b/app/controllers/public_body_controller.rb @@ -7,7 +7,7 @@ # # $Id: public_body_controller.rb,v 1.8 2009-09-14 13:27:00 francis Exp $ -require 'csv' +require 'fastercsv' class PublicBodyController < ApplicationController # XXX tidy this up with better error messages, and a more standard infrastructure for the redirect to canonical URL @@ -148,10 +148,10 @@ class PublicBodyController < ApplicationController end def list_all_csv - public_bodies = PublicBody.find(:all, :order => 'url_name') - report = StringIO.new - CSV::Writer.generate(report, ',') do |title| - title << [ + public_bodies = PublicBody.find(:all, :order => 'url_name', + :include => [:translations, :tags]) + report = FasterCSV.generate() do |csv| + csv << [ 'Name', 'Short name', # deliberately not including 'Request email' @@ -164,7 +164,7 @@ class PublicBodyController < ApplicationController 'Version', ] public_bodies.each do |public_body| - title << [ + csv << [ public_body.name, public_body.short_name, # DO NOT include request_email (we don't want to make it @@ -179,8 +179,7 @@ class PublicBodyController < ApplicationController ] end end - report.rewind - send_data(report.read, :type=> 'text/csv; charset=utf-8; header=present', + send_data(report, :type=> 'text/csv; charset=utf-8; header=present', :filename => 'all-authorities.csv', :disposition =>'attachment', :encoding => 'utf8') end diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index 6e983a014..268ecc73a 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -422,19 +422,19 @@ class RequestController < ApplicationController old_described_state = @info_request.described_state @info_request.set_described_state(params[:incoming_message][:described_state]) - # If you're not the *actual* requester owner. e.g. you are playing the + # If you're not the *actual* requester. e.g. you are playing the # classification game, or you're doing this just because you are an # admin user (not because you also own the request). if !@info_request.is_actual_owning_user?(authenticated_user) - # Log what you did, for classification game score purposes. We - # don't log if you were the requester XXX This is presumably so you - # don't score for classifying your own requests. Could instead - # always log and filter at display time. - @info_request.log_event("status_update", + # Log the status change by someone other than the requester + event = @info_request.log_event("status_update", { :user_id => authenticated_user.id, :old_described_state => old_described_state, :described_state => @info_request.described_state, }) + # Create a classification event for league tables + RequestClassification.create!(:user_id => authenticated_user.id, + :info_request_event_id => event.id) # Don't give advice on what to do next, as it isn't their request RequestMailer.deliver_old_unclassified_updated(@info_request) if !@info_request.is_external? diff --git a/app/controllers/request_game_controller.rb b/app/controllers/request_game_controller.rb index 904c44759..f22652dd1 100644 --- a/app/controllers/request_game_controller.rb +++ b/app/controllers/request_game_controller.rb @@ -11,13 +11,12 @@ class RequestGameController < ApplicationController def play session[:request_game] = Time.now - old = InfoRequest.find_old_unclassified(:conditions => ["prominence = 'normal'"]) - @missing = old.size + @missing = InfoRequest.count_old_unclassified(:conditions => ["prominence = 'normal'"]) @total = InfoRequest.count @done = @total - @missing @percentage = (@done.to_f / @total.to_f * 10000).round / 100.0 - @requests = old.sort_by{ rand }.slice(0..2) + @requests = InfoRequest.get_random_old_unclassified(3) if @missing == 0 flash[:notice] = _('<p>All done! Thank you very much for your help.</p><p>There are <a href="{{helpus_url}}">more things you can do</a> to help {{site_name}}.</p>', @@ -25,12 +24,8 @@ class RequestGameController < ApplicationController :site_name => site_name) end - @league_table_28_days = InfoRequestEvent.make_league_table( - [ "event_type = 'status_update' and created_at >= ?", Time.now() - 28.days ] - )[0..10] - @league_table_all_time = InfoRequestEvent.make_league_table( - [ "event_type = 'status_update'"] - )[0..10] + @league_table_28_days = RequestClassification.league_table(10, [ "created_at >= ?", Time.now() - 28.days ]) + @league_table_all_time = RequestClassification.league_table(10) @play_urls = true end diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 6f472c290..2e16d0f58 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -35,7 +35,7 @@ class InfoRequest < ActiveRecord::Base belongs_to :user validate :must_be_internal_or_external - belongs_to :public_body + belongs_to :public_body, :counter_cache => true validates_presence_of :public_body_id has_many :outgoing_messages, :order => 'created_at' @@ -223,7 +223,7 @@ class InfoRequest < ActiveRecord::Base incoming_message.clear_in_database_caches! end end - + # For debugging def InfoRequest.profile_search(query) t = Time.now.usec @@ -939,26 +939,54 @@ public # Used to find when event last changed def InfoRequest.last_event_time_clause(event_type=nil) event_type_clause = '' - event_type_clause = " and info_request_events.event_type = '#{event_type}'" if event_type - "(select created_at from info_request_events where info_request_events.info_request_id = info_requests.id#{event_type_clause} order by created_at desc limit 1)" + event_type_clause = " AND info_request_events.event_type = '#{event_type}'" if event_type + "(SELECT created_at + FROM info_request_events + WHERE info_request_events.info_request_id = info_requests.id + #{event_type_clause} + ORDER BY created_at desc + LIMIT 1)" end - def InfoRequest.find_old_unclassified(extra_params={}) + def InfoRequest.old_unclassified_params(extra_params, include_last_response_time=false) last_response_created_at = last_event_time_clause('response') age = extra_params[:age_in_days] ? extra_params[:age_in_days].days : OLD_AGE_IN_DAYS - params = {:select => "*, #{last_response_created_at} as last_response_time", - :conditions => ["awaiting_description = ? and #{last_response_created_at} < ? and url_title != 'holding_pen' and user_id is not null", - true, Time.now() - age], - :order => "last_response_time"} - params[:limit] = extra_params[:limit] if extra_params[:limit] - params[:include] = extra_params[:include] if extra_params[:include] + params = { :conditions => ["awaiting_description = ? + AND #{last_response_created_at} < ? + AND url_title != 'holding_pen' + AND user_id IS NOT NULL", + true, Time.now() - age] } + if include_last_response_time + params[:select] = "*, #{last_response_created_at} AS last_response_time" + params[:order] = 'last_response_time' + end + return params + end + + def InfoRequest.count_old_unclassified(extra_params={}) + params = old_unclassified_params(extra_params) + count(:all, params) + end + + def InfoRequest.get_random_old_unclassified(limit) + params = old_unclassified_params({}) + params[:limit] = limit + params[:order] = "random()" + find(:all, params) + end + + def InfoRequest.find_old_unclassified(extra_params={}) + params = old_unclassified_params(extra_params, include_last_response_time=true) + [:limit, :include, :offset].each do |extra| + params[extra] = extra_params[extra] if extra_params[extra] + end if extra_params[:order] params[:order] = extra_params[:order] params.delete(:select) end if extra_params[:conditions] condition_string = extra_params[:conditions].shift - params[:conditions][0] += " and #{condition_string}" + params[:conditions][0] += " AND #{condition_string}" params[:conditions] += extra_params[:conditions] end find(:all, params) diff --git a/app/models/info_request_event.rb b/app/models/info_request_event.rb index a827d19a4..54d2f5ef7 100644 --- a/app/models/info_request_event.rb +++ b/app/models/info_request_event.rb @@ -43,7 +43,7 @@ class InfoRequestEvent < ActiveRecord::Base 'resent', 'followup_sent', 'followup_resent', - + 'edit', # title etc. edited (in admin interface) 'edit_outgoing', # outgoing message edited (in admin interface) 'edit_comment', # comment edited (in admin interface) @@ -53,7 +53,7 @@ class InfoRequestEvent < ActiveRecord::Base 'move_request', # changed user or public body (in admin interface) 'hide', # hid a request (in admin interface) 'manual', # you did something in the db by hand - + 'response', 'comment', 'status_update' @@ -389,24 +389,6 @@ class InfoRequestEvent < ActiveRecord::Base return TMail::Address.parse(prev_addr).address == TMail::Address.parse(curr_addr).address end - # Given a find condition clause, creates a league table of users who made those events. - # XXX this isn't very generic yet, it is just used for the categorisation game tables. - def InfoRequestEvent.make_league_table(conditions) - status_update_events = InfoRequestEvent.find(:all, :conditions => conditions) - table = Hash.new { |h,k| h[k] = 0 } - for event in status_update_events - user_id = event.params[:user_id] - table[user_id] += 1 - end - league_table = [] - for user_id, count in table - user = User.find(user_id) - league_table.push([user, count]) - end - league_table.sort! { |a,b| b[1] <=> a[1] } - return league_table - end - def json_for_api(deep, snippet_highlight_proc = nil) ret = { :id => self.id, diff --git a/app/models/public_body.rb b/app/models/public_body.rb index fb30da234..77da81d4c 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -45,6 +45,8 @@ class PublicBody < ActiveRecord::Base has_many :censor_rules, :order => 'created_at desc' has_tag_string + before_save :set_api_key, :set_default_publication_scheme + translates :name, :short_name, :request_email, :url_name, :notes, :first_letter, :publication_scheme @@ -89,13 +91,13 @@ class PublicBody < ActiveRecord::Base end end - def after_initialize + def set_default_publication_scheme # Make sure publication_scheme gets the correct default value. # (This would work automatically, were publication_scheme not a translated attribute) self.publication_scheme = "" if self.publication_scheme.nil? end - def before_save + def set_api_key self.api_key = SecureRandom.base64(33) if self.api_key.nil? end @@ -184,7 +186,7 @@ class PublicBody < ActiveRecord::Base end acts_as_versioned - self.non_versioned_columns << 'created_at' << 'updated_at' << 'first_letter' << 'api_key' + self.non_versioned_columns << 'created_at' << 'updated_at' << 'first_letter' << 'api_key' << 'info_requests_count' class Version attr_accessor :created_at @@ -549,9 +551,10 @@ class PublicBody < ActiveRecord::Base def notes_as_html self.notes end + def notes_without_html # assume notes are reasonably behaved HTML, so just use simple regexp on this - self.notes.nil? ? '' : self.notes.gsub(/<\/?[^>]*>/, "") + @notes_without_html ||= (self.notes.nil? ? '' : self.notes.gsub(/<\/?[^>]*>/, "")) end def json_for_api diff --git a/app/models/request_classification.rb b/app/models/request_classification.rb new file mode 100644 index 000000000..678b6cd16 --- /dev/null +++ b/app/models/request_classification.rb @@ -0,0 +1,16 @@ +class RequestClassification < ActiveRecord::Base + belongs_to :user + + # return classification instances representing the top n + # users, with a 'cnt' attribute representing the number + # of classifications the user has made. + def RequestClassification.league_table(size, conditions=[]) + find(:all, :select => 'user_id, count(*) as cnt', + :conditions => conditions, + :group => 'user_id', + :order => 'cnt desc', + :limit => size, + :include => :user) + end + +end
\ No newline at end of file diff --git a/app/views/admin_request/list_old_unclassified.rhtml b/app/views/admin_request/list_old_unclassified.rhtml index f42ed0d43..2e75c2174 100644 --- a/app/views/admin_request/list_old_unclassified.rhtml +++ b/app/views/admin_request/list_old_unclassified.rhtml @@ -6,10 +6,11 @@ <ul> <% for @request in @info_requests %> <li> - <%= request_both_links(@request) %> + <%= request_both_links(@request) %> – <%=simple_date(@request.get_last_response_event.created_at)%> </li> <% end %> </ul> +<%= will_paginate(@info_requests) %> diff --git a/app/views/request_game/play.rhtml b/app/views/request_game/play.rhtml index acf6cce87..eedf19ca2 100644 --- a/app/views/request_game/play.rhtml +++ b/app/views/request_game/play.rhtml @@ -7,22 +7,22 @@ </p> <h2>Top recent players</h2> <table> - <% c = 0; for user, count in @league_table_28_days %> + <% c = 0; for classifications in @league_table_28_days %> <tr> <td> <%= c += 1 %>. <td> - <td> <%= user_link(user) %> </td> - <td> <%=pluralize(count, 'request').gsub(" ", " ")%> </td> + <td> <%= user_link(classifications.user) %> </td> + <td> <%=pluralize(classifications.cnt, 'request').gsub(" ", " ")%> </td> </tr> <% end %> </table> <h2>All time best players</h2> <table> - <% c = 0; for user, count in @league_table_all_time %> + <% c = 0; for classifications in @league_table_all_time %> <tr> <td> <%= c += 1 %>. <td> - <td> <%= user_link(user) %> </td> - <td> <%=pluralize(count, 'request').gsub(" ", " ")%> </td> + <td> <%= user_link(classifications.user) %> </td> + <td> <%= pluralize(classifications.cnt, 'request').gsub(" ", " ")%> </td> </tr> <% end %> </table> diff --git a/config/.gitignore b/config/.gitignore index 28efb03ec..78d586ea8 100644 --- a/config/.gitignore +++ b/config/.gitignore @@ -1,8 +1,9 @@ -/config.tmp -/general -/general.yml -/database.yml -/rails_env.rb -/logrotate -/memcached.yml -/*.deployed +config.tmp +general +general.yml +database.yml +rails_env.rb +logrotate +memcached.yml +*.deployed +deploy.yml diff --git a/config/deploy.rb b/config/deploy.rb new file mode 100644 index 000000000..888710f83 --- /dev/null +++ b/config/deploy.rb @@ -0,0 +1,69 @@ +require 'bundler/capistrano' + +set :stage, 'staging' unless exists? :stage + +configuration = YAML.load_file('config/deploy.yml')[stage] + +set :application, 'alaveteli' +set :scm, :git +set :deploy_via, :remote_cache +set :repository, configuration['repository'] +set :branch, configuration['branch'] +set :git_enable_submodules, true +set :deploy_to, configuration['deploy_to'] +set :user, configuration['user'] +set :use_sudo, false + +server configuration['server'], :app, :web, :db, :primary => true + +namespace :rake do + namespace :themes do + task :install do + run "cd #{release_path} && bundle exec rake themes:install RAILS_ENV=#{rails_env}" + end + end +end + +# Not in the rake namespace because we're also specifying app-specific arguments here +namespace :xapian do + desc 'Rebuilds the Xapian index as per the ./scripts/rebuild-xapian-index script' + task :rebuild_index do + run "cd #{current_path} && bundle exec rake xapian:rebuild_index models='PublicBody User InfoRequestEvent' RAILS_ENV=#{rails_env}" + end +end + +namespace :deploy do + desc "Restarting mod_rails with restart.txt" + task :restart, :roles => :app, :except => { :no_release => true } do + run "touch #{current_path}/tmp/restart.txt" + end + + [:start, :stop].each do |t| + desc "#{t} task is a no-op with mod_rails" + task t, :roles => :app do ; end + end + + desc 'Link configuration after a code update' + task :symlink_configuration do + links = { + "#{release_path}/config/database.yml" => "#{shared_path}/database.yml", + "#{release_path}/config/general.yml" => "#{shared_path}/general.yml", + "#{release_path}/files" => "#{shared_path}/files", + "#{release_path}/cache" => "#{shared_path}/cache", + "#{release_path}/vendor/plugins/acts_as_xapian/xapiandbs" => "#{shared_path}/xapiandbs", + "#{release_path}/public/download" => "#{release_path}/cache/zips/download" + } + + # "ln -sf <a> <b>" creates a symbolic link but deletes <b> if it already exists + run links.map {|a| "ln -sf #{a.last} #{a.first}"}.join(";") + end + + after 'deploy:setup' do + run "mkdir -p #{shared_path}/files" + run "mkdir -p #{shared_path}/cache" + run "mkdir -p #{shared_path}/xapiandbs" + end +end + +after 'deploy:update_code', 'deploy:symlink_configuration' +after 'deploy:update_code', 'rake:themes:install' diff --git a/config/deploy.yml.example b/config/deploy.yml.example new file mode 100644 index 000000000..aea045dff --- /dev/null +++ b/config/deploy.yml.example @@ -0,0 +1,13 @@ +# Site-specific deployment configuration lives in this file +production: + repository: git://github.com:mysociety/alaveteli.git + branch: master + server: www.example.com + user: deploy + deploy_to: /srv/www/alaveteli_production +staging: + repository: git://github.com:mysociety/alaveteli.git + branch: develop + server: test.example.com + user: deploy + deploy_to: /srv/www/alaveteli_staging diff --git a/db/migrate/20120910153022_create_request_classifications.rb b/db/migrate/20120910153022_create_request_classifications.rb new file mode 100644 index 000000000..7c6270c9e --- /dev/null +++ b/db/migrate/20120910153022_create_request_classifications.rb @@ -0,0 +1,14 @@ +class CreateRequestClassifications < ActiveRecord::Migration + def self.up + create_table :request_classifications do |t| + t.integer :user_id + t.integer :info_request_event_id + t.timestamps + end + add_index :request_classifications, :user_id + end + + def self.down + drop_table :request_classifications + end +end diff --git a/db/migrate/20120912111713_add_raw_email_index_to_incoming_messages.rb b/db/migrate/20120912111713_add_raw_email_index_to_incoming_messages.rb new file mode 100644 index 000000000..14174935e --- /dev/null +++ b/db/migrate/20120912111713_add_raw_email_index_to_incoming_messages.rb @@ -0,0 +1,9 @@ +class AddRawEmailIndexToIncomingMessages < ActiveRecord::Migration + def self.up + add_index :incoming_messages, :raw_email_id + end + + def self.down + remove_index :incoming_messages, :raw_email_id + end +end diff --git a/db/migrate/20120912112036_add_info_request_id_index_to_exim_logs.rb b/db/migrate/20120912112036_add_info_request_id_index_to_exim_logs.rb new file mode 100644 index 000000000..81e2a7946 --- /dev/null +++ b/db/migrate/20120912112036_add_info_request_id_index_to_exim_logs.rb @@ -0,0 +1,9 @@ +class AddInfoRequestIdIndexToEximLogs < ActiveRecord::Migration + def self.up + add_index :exim_logs, :info_request_id + end + + def self.down + remove_index :exim_logs, :info_request_id + end +end diff --git a/db/migrate/20120912112312_add_info_request_id_index_to_incoming_and_outgoing_messages.rb b/db/migrate/20120912112312_add_info_request_id_index_to_incoming_and_outgoing_messages.rb new file mode 100644 index 000000000..814fa7540 --- /dev/null +++ b/db/migrate/20120912112312_add_info_request_id_index_to_incoming_and_outgoing_messages.rb @@ -0,0 +1,11 @@ +class AddInfoRequestIdIndexToIncomingAndOutgoingMessages < ActiveRecord::Migration + def self.up + add_index :incoming_messages, :info_request_id + add_index :outgoing_messages, :info_request_id + end + + def self.down + remove_index :incoming_messages, :info_request_id + remove_index :outgoing_messages, :info_request_id + end +end diff --git a/db/migrate/20120912112655_add_incoming_message_id_index_to_foi_attachments.rb b/db/migrate/20120912112655_add_incoming_message_id_index_to_foi_attachments.rb new file mode 100644 index 000000000..be0bf76c3 --- /dev/null +++ b/db/migrate/20120912112655_add_incoming_message_id_index_to_foi_attachments.rb @@ -0,0 +1,9 @@ +class AddIncomingMessageIdIndexToFoiAttachments < ActiveRecord::Migration + def self.up + add_index :foi_attachments, :incoming_message_id + end + + def self.down + remove_index :foi_attachments, :incoming_message_id + end +end diff --git a/db/migrate/20120912113004_add_indexes_to_info_request_events.rb b/db/migrate/20120912113004_add_indexes_to_info_request_events.rb new file mode 100644 index 000000000..b3780322f --- /dev/null +++ b/db/migrate/20120912113004_add_indexes_to_info_request_events.rb @@ -0,0 +1,13 @@ +class AddIndexesToInfoRequestEvents < ActiveRecord::Migration + def self.up + add_index :info_request_events, :incoming_message_id + add_index :info_request_events, :outgoing_message_id + add_index :info_request_events, :comment_id + end + + def self.down + remove_index :info_request_events, :incoming_message_id + remove_index :info_request_events, :outgoing_message_id + remove_index :info_request_events, :comment_id + end +end diff --git a/db/migrate/20120912113720_add_public_body_index_to_info_requests.rb b/db/migrate/20120912113720_add_public_body_index_to_info_requests.rb new file mode 100644 index 000000000..a56cad1f9 --- /dev/null +++ b/db/migrate/20120912113720_add_public_body_index_to_info_requests.rb @@ -0,0 +1,9 @@ +class AddPublicBodyIndexToInfoRequests < ActiveRecord::Migration + def self.up + add_index :info_requests, :public_body_id + end + + def self.down + remove_index :info_requests, :public_body_id + end +end diff --git a/db/migrate/20120912114022_add_user_index_to_info_requests.rb b/db/migrate/20120912114022_add_user_index_to_info_requests.rb new file mode 100644 index 000000000..8be51c0c8 --- /dev/null +++ b/db/migrate/20120912114022_add_user_index_to_info_requests.rb @@ -0,0 +1,9 @@ +class AddUserIndexToInfoRequests < ActiveRecord::Migration + def self.up + add_index :info_requests, :user_id + end + + def self.down + remove_index :info_requests, :user_id + end +end diff --git a/db/migrate/20120912170035_add_info_requests_count_to_public_bodies.rb b/db/migrate/20120912170035_add_info_requests_count_to_public_bodies.rb new file mode 100644 index 000000000..d77dbaa64 --- /dev/null +++ b/db/migrate/20120912170035_add_info_requests_count_to_public_bodies.rb @@ -0,0 +1,17 @@ +class AddInfoRequestsCountToPublicBodies < ActiveRecord::Migration + def self.up + add_column :public_bodies, :info_requests_count, :integer, :null => false, :default => 0 + + PublicBody.reset_column_information + + PublicBody.find_each do |public_body| + public_body.update_attribute :info_requests_count, public_body.info_requests.length + end + + end + + def self.down + remove_column :public_bodies, :info_requests_count + end + +end diff --git a/db/migrate/20120913074940_add_incoming_message_index_to_outgoing_messages.rb b/db/migrate/20120913074940_add_incoming_message_index_to_outgoing_messages.rb new file mode 100644 index 000000000..893395f41 --- /dev/null +++ b/db/migrate/20120913074940_add_incoming_message_index_to_outgoing_messages.rb @@ -0,0 +1,9 @@ +class AddIncomingMessageIndexToOutgoingMessages < ActiveRecord::Migration + def self.up + add_index :outgoing_messages, :incoming_message_followup_id + end + + def self.down + remove_index :outgoing_messages, :incoming_message_followup_id + end +end diff --git a/db/migrate/20120913080807_add_info_request_event_index_to_track_things_sent_emails.rb b/db/migrate/20120913080807_add_info_request_event_index_to_track_things_sent_emails.rb new file mode 100644 index 000000000..d119f55b3 --- /dev/null +++ b/db/migrate/20120913080807_add_info_request_event_index_to_track_things_sent_emails.rb @@ -0,0 +1,9 @@ +class AddInfoRequestEventIndexToTrackThingsSentEmails < ActiveRecord::Migration + def self.up + add_index :track_things_sent_emails, :info_request_event_id + end + + def self.down + remove_index :track_things_sent_emails, :info_request_event_id + end +end diff --git a/db/migrate/20120913081136_add_info_request_event_index_to_user_info_request_sent_alerts.rb b/db/migrate/20120913081136_add_info_request_event_index_to_user_info_request_sent_alerts.rb new file mode 100644 index 000000000..aa9f404f7 --- /dev/null +++ b/db/migrate/20120913081136_add_info_request_event_index_to_user_info_request_sent_alerts.rb @@ -0,0 +1,9 @@ +class AddInfoRequestEventIndexToUserInfoRequestSentAlerts < ActiveRecord::Migration + def self.up + add_index :user_info_request_sent_alerts, :info_request_event_id + end + + def self.down + remove_index :user_info_request_sent_alerts, :info_request_event_id + end +end diff --git a/doc/DEPLOY.md b/doc/DEPLOY.md new file mode 100644 index 000000000..adeb0e113 --- /dev/null +++ b/doc/DEPLOY.md @@ -0,0 +1,41 @@ +# Deployment + +mySociety uses a custom deployment and buildout system however Capistrano is included as part of Alaveteli as a standard deployment system. + +## Capistrano + +### Set up + +First you need to customise your deployment settings, e.g. the name of the server you're deploying to. This is done by copying the example file `config/deploy.yml.example` to `config/deploy.yml` and editing the settings to suit you. + +TODO: The following instructions could be greatly improved + +These are the general steps required to get your staging server up and running: + +* Install packages from `config/packages` +* Install Postgres and configure a user +* Create a directory to deploy to and make sure your deployment user can write to it +* Run `cap deploy:setup` to create directories, etc. +* Run `cap deploy:update_code` so that we've got a copy of the example config on the server. This process will take a long time installing gems, etc. it will also fail on `rake:themes:install` but that's OK +* SSH to the server, change to the `deploy_to` directory +* `cp releases/[SOME_DATE]/config/general.yml-example shared/general.yml` +* `cp releases/[SOME_DATE]/config/general.yml-example shared/general.yml` +* Edit those files to match your required settings +* Back on your machine run `cap deploy` and it should successfully deploy +* Run the DB migrations `cap deploy:migrate` +* Build the Xapian DB `cap xapian:rebuild_index` +* Configure Apache/Passenger with a DocumentRoot of `your_deploy_to/current/public` +* Phew. Time to admire your work by browsing to the server! + +### Usage + +Ensure you've got a `config/deploy.yml` file with the correct settings for your site. You'll need to share this with everyone in your team that deploys so it might be a good idea to keep the latest version in a [Gist](http://gist.github.com/). + +To deploy to staging just run `cap deploy` but if you want to deploy to production you need to run `cap -S stage=production deploy`. + +For additional usage instructions, see the [Capistrano wiki](https://github.com/capistrano/capistrano/wiki/). + +### TODO + +* Get `cap deploy:setup` to do most of the work described above in the *Set up* section +* Use [Whenever](https://github.com/javan/whenever) to set up cronjobs diff --git a/lib/tasks/temp.rake b/lib/tasks/temp.rake index 9decc13db..e49a84ecb 100644 --- a/lib/tasks/temp.rake +++ b/lib/tasks/temp.rake @@ -1,5 +1,14 @@ namespace :temp do + desc 'Populate the request_classifications table from info_request_events' + task :populate_request_classifications => :environment do + InfoRequestEvent.find_each(:conditions => ["event_type = 'status_update'"]) do |classification| + RequestClassification.create!(:created_at => classification.created_at, + :user_id => classification.params[:user_id], + :info_request_event_id => classification.id) + end + end + desc "Remove plaintext passwords from post_redirect params" task :remove_post_redirect_passwords => :environment do PostRedirect.find_each(:conditions => ['post_params_yaml is not null']) do |post_redirect| diff --git a/lib/tasks/themes.rake b/lib/tasks/themes.rake index 2b1dbb3a9..6eb64b4b0 100644 --- a/lib/tasks/themes.rake +++ b/lib/tasks/themes.rake @@ -9,33 +9,43 @@ namespace :themes do File.join(plugin_dir, theme_name) end + def checkout_tag(version) + checkout_command = "git checkout #{usage_tag(version)}" + success = system(checkout_command) + puts "Using tag #{usage_tag(version)}" if verbose && success + success + end + + def usage_tag(version) + "use-with-alaveteli-#{version}" + end + def install_theme_using_git(name, uri, verbose=false, options={}) - mkdir_p(install_path = theme_dir(name)) - Dir.chdir install_path do - init_cmd = "git init" - init_cmd += " -q" if options[:quiet] and not verbose - puts init_cmd if verbose - system(init_cmd) - base_cmd = "git pull --depth 1 #{uri}" - # Is there a tag for this version of Alaveteli? - usage_tag = "use-with-alaveteli-#{ALAVETELI_VERSION}" - # Query the remote repository passing flags for tags - version_tag = `git ls-remote --tags #{uri} #{usage_tag}` - if /^[a-z0-9]+\s+refs\/tags\/#{Regexp.escape(usage_tag)}$/.match(version_tag) - # If we got a tag, pull that instead of HEAD - puts "Using tag #{usage_tag}" if verbose - base_cmd += " refs/tags/#{usage_tag}" - else - puts "No specific tag for this version: using HEAD" if verbose - end - base_cmd += " -q" if options[:quiet] and not verbose - puts base_cmd if verbose - if system(base_cmd) - puts "removing: .git .gitignore" if verbose - rm_rf %w(.git .gitignore) + install_path = theme_dir(name) + Dir.chdir(plugin_dir) do + clone_command = "git clone #{uri} #{name}" + if system(clone_command) + Dir.chdir install_path do + # try to checkout a tag exactly matching ALAVETELI VERSION + tag_checked_out = checkout_tag(ALAVETELI_VERSION) + if ! tag_checked_out + # if we're on a hotfix release (four sequence elements or more), + # look for a usage tag matching the minor release (three sequence elements) + # and check that out if found + if hotfix_version = /^(\d+\.\d+\.\d+)(\.\d+)+/.match(ALAVETELI_VERSION) + base_version = hotfix_version[1] + tag_checked_out = checkout_tag(base_version) + end + end + if ! tag_checked_out + puts "No specific tag for this version: using HEAD" if verbose + end + puts "removing: .git .gitignore" if verbose + rm_rf %w(.git .gitignore) + end else rm_rf install_path - raise "#{base_cmd} failed! Stopping." + raise "#{clone_command} failed! Stopping." end end end diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index b8425613c..95737a250 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -1275,7 +1275,8 @@ describe RequestController, "when classifying an information request" do expected_params = {:user_id => users(:silly_name_user).id, :old_described_state => 'waiting_response', :described_state => 'rejected'} - @dog_request.should_receive(:log_event).with("status_update", expected_params) + event = mock_model(InfoRequestEvent) + @dog_request.should_receive(:log_event).with("status_update", expected_params).and_return(event) post_status('rejected') end @@ -1314,10 +1315,19 @@ describe RequestController, "when classifying an information request" do end it 'should log a status update event' do + event = mock_model(InfoRequestEvent) expected_params = {:user_id => @admin_user.id, :old_described_state => 'waiting_response', :described_state => 'rejected'} - @dog_request.should_receive(:log_event).with("status_update", expected_params) + @dog_request.should_receive(:log_event).with("status_update", expected_params).and_return(event) + post_status('rejected') + end + + it 'should record a classification' do + event = mock_model(InfoRequestEvent) + @dog_request.stub!(:log_event).with("status_update", anything()).and_return(event) + RequestClassification.should_receive(:create!).with(:user_id => @admin_user.id, + :info_request_event_id => event.id) post_status('rejected') end diff --git a/spec/fixtures/public_bodies.yml b/spec/fixtures/public_bodies.yml index 367e0fc50..615c4bcb6 100644 --- a/spec/fixtures/public_bodies.yml +++ b/spec/fixtures/public_bodies.yml @@ -1,4 +1,4 @@ -geraldine_public_body: +geraldine_public_body: name: The Geraldine Quango first_letter: T updated_at: 2007-10-24 10:51:01.161639 @@ -11,7 +11,8 @@ geraldine_public_body: url_name: tgq created_at: 2007-10-24 10:51:01.161639 api_key: 1 -humpadink_public_body: + info_requests_count: 4 +humpadink_public_body: name: "Department for Humpadinking" first_letter: D updated_at: 2007-10-25 10:51:01.161639 @@ -25,6 +26,7 @@ humpadink_public_body: created_at: 2007-10-25 10:51:01.161639 notes: An albatross told me!!! api_key: 2 + info_requests_count: 2 forlorn_public_body: name: "Department of Loneliness" first_letter: D @@ -39,7 +41,8 @@ forlorn_public_body: created_at: 2011-01-26 14:11:02.12345 notes: A very lonely public body that no one has corresponded with api_key: 3 -silly_walks_public_body: + info_requests_count: 0 +silly_walks_public_body: id: 5 version: 1 name: "Ministry of Silly Walks" @@ -53,7 +56,8 @@ silly_walks_public_body: created_at: 2007-10-25 10:51:01.161639 notes: You know the one. api_key: 4 -sensible_walks_public_body: + info_requests_count: 2 +sensible_walks_public_body: id: 6 version: 1 name: "Ministry of Sensible Walks" @@ -67,4 +71,4 @@ sensible_walks_public_body: last_edit_editor: robin created_at: 2008-10-25 10:51:01.161639 api_key: 5 - + info_requests_count: 1 diff --git a/spec/fixtures/public_body_translations.yml b/spec/fixtures/public_body_translations.yml index cbb55bb0c..d705358c5 100644 --- a/spec/fixtures/public_body_translations.yml +++ b/spec/fixtures/public_body_translations.yml @@ -1,4 +1,4 @@ -geraldine_es_public_body_translation: +geraldine_es_public_body_translation: name: El A Geraldine Quango first_letter: E request_email: geraldine-requests@localhost @@ -8,8 +8,9 @@ geraldine_es_public_body_translation: url_name: etgq locale: es notes: "" + publication_scheme: "" -geraldine_en_public_body_translation: +geraldine_en_public_body_translation: name: Geraldine Quango first_letter: G request_email: geraldine-requests@localhost @@ -19,8 +20,9 @@ geraldine_en_public_body_translation: url_name: tgq locale: en notes: "" + publication_scheme: "" -humpadink_es_public_body_translation: +humpadink_es_public_body_translation: name: "El Department for Humpadinking" first_letter: E request_email: humpadink-requests@localhost @@ -30,8 +32,9 @@ humpadink_es_public_body_translation: url_name: edfh locale: es notes: Baguette + publication_scheme: "" -humpadink_en_public_body_translation: +humpadink_en_public_body_translation: name: "Department for Humpadinking" first_letter: D request_email: humpadink-requests@localhost @@ -41,8 +44,9 @@ humpadink_en_public_body_translation: url_name: dfh locale: en notes: An albatross told me!!! + publication_scheme: "" -forlorn_en_public_body_translation: +forlorn_en_public_body_translation: name: "Department of Loneliness" first_letter: D request_email: forlorn-requests@localhost diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index c55127992..204c600d9 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -341,6 +341,14 @@ describe InfoRequest do InfoRequest.find_old_unclassified(:limit => 5) end + it 'should ask for requests using any offset param supplied' do + InfoRequest.should_receive(:find).with(:all, {:select => anything, + :order => anything, + :conditions=> anything, + :offset => 100}) + InfoRequest.find_old_unclassified(:offset => 100) + end + it 'should not limit the number of requests returned by default' do InfoRequest.should_not_receive(:find).with(:all, {:select => anything, :order => anything, @@ -350,20 +358,49 @@ describe InfoRequest do end it 'should add extra conditions if supplied' do - InfoRequest.should_receive(:find).with(:all, - {:select=> anything, - :order=> anything, - :conditions=>["awaiting_description = ? and (select created_at from info_request_events where info_request_events.info_request_id = info_requests.id and info_request_events.event_type = 'response' order by created_at desc limit 1) < ? and url_title != 'holding_pen' and user_id is not null and prominence != 'backpage'", - true, Time.now - 21.days]}) + expected_conditions = ["awaiting_description = ? + AND (SELECT created_at + FROM info_request_events + WHERE info_request_events.info_request_id = info_requests.id + AND info_request_events.event_type = 'response' + ORDER BY created_at desc LIMIT 1) < ? + AND url_title != 'holding_pen' + AND user_id IS NOT NULL + AND prominence != 'backpage'".split(' ').join(' '), + true, Time.now - 21.days] + # compare conditions ignoring whitespace differences + InfoRequest.should_receive(:find) do |all, query_params| + query_string = query_params[:conditions][0] + query_params[:conditions][0] = query_string.split(' ').join(' ') + query_params[:conditions].should == expected_conditions + end InfoRequest.find_old_unclassified({:conditions => ["prominence != 'backpage'"]}) end - it 'should ask the database for requests that are awaiting description, have a last response older than 21 days old, are not the holding pen and are not backpaged' do - InfoRequest.should_receive(:find).with(:all, - {:select=>"*, (select created_at from info_request_events where info_request_events.info_request_id = info_requests.id and info_request_events.event_type = 'response' order by created_at desc limit 1) as last_response_time", - :order=>"last_response_time", - :conditions=>["awaiting_description = ? and (select created_at from info_request_events where info_request_events.info_request_id = info_requests.id and info_request_events.event_type = 'response' order by created_at desc limit 1) < ? and url_title != 'holding_pen' and user_id is not null", - true, Time.now - 21.days]}) + it 'should ask the database for requests that are awaiting description, have a last response older + than 21 days old, have a user, are not the holding pen and are not backpaged' do + expected_conditions = ["awaiting_description = ? + AND (SELECT created_at + FROM info_request_events + WHERE info_request_events.info_request_id = info_requests.id + AND info_request_events.event_type = 'response' + ORDER BY created_at desc LIMIT 1) < ? + AND url_title != 'holding_pen' + AND user_id IS NOT NULL".split(' ').join(' '), + true, Time.now - 21.days] + expected_select = "*, (SELECT created_at + FROM info_request_events + WHERE info_request_events.info_request_id = info_requests.id + AND info_request_events.event_type = 'response' + ORDER BY created_at desc LIMIT 1) + AS last_response_time".split(' ').join(' ') + InfoRequest.should_receive(:find) do |all, query_params| + query_string = query_params[:conditions][0] + query_params[:conditions][0] = query_string.split(' ').join(' ') + query_params[:conditions].should == expected_conditions + query_params[:select].split(' ').join(' ').should == expected_select + query_params[:order].should == "last_response_time" + end InfoRequest.find_old_unclassified end diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb index 9e22a9c43..011824190 100644 --- a/spec/models/public_body_spec.rb +++ b/spec/models/public_body_spec.rb @@ -407,6 +407,7 @@ describe PublicBody, " when loading CSV files" do end describe PublicBody do + describe "calculated home page" do it "should return the home page verbatim if it's present" do public_body = PublicBody.new @@ -438,4 +439,17 @@ describe PublicBody do public_body.calculated_home_page.should == "https://example.com" end end + + describe 'when asked for notes without html' do + + before do + @public_body = PublicBody.new(:notes => 'some <a href="/notes">notes</a>') + end + + it 'should remove simple tags from notes' do + @public_body.notes_without_html.should == 'some notes' + end + + end + end diff --git a/spec/models/request_mailer_spec.rb b/spec/models/request_mailer_spec.rb index ea75ec765..98681a9e9 100644 --- a/spec/models/request_mailer_spec.rb +++ b/spec/models/request_mailer_spec.rb @@ -29,7 +29,7 @@ describe RequestMailer, " when receiving incoming mail" do InfoRequest.holding_pen_request.incoming_messages.size.should == 1 last_event = InfoRequest.holding_pen_request.incoming_messages[0].info_request.get_last_event last_event.params[:rejected_reason].should == "Could not identify the request from the email address" - + deliveries = ActionMailer::Base.deliveries deliveries.size.should == 1 mail = deliveries[0] @@ -49,7 +49,7 @@ describe RequestMailer, " when receiving incoming mail" do InfoRequest.holding_pen_request.incoming_messages.size.should == 1 last_event = InfoRequest.holding_pen_request.incoming_messages[0].info_request.get_last_event last_event.params[:rejected_reason].should =~ /there is no "From" address/ - + deliveries = ActionMailer::Base.deliveries deliveries.size.should == 1 mail = deliveries[0] @@ -69,7 +69,7 @@ describe RequestMailer, " when receiving incoming mail" do InfoRequest.holding_pen_request.incoming_messages.size.should == 1 last_event = InfoRequest.holding_pen_request.incoming_messages[0].info_request.get_last_event last_event.params[:rejected_reason].should =~ /Only the authority can reply/ - + deliveries = ActionMailer::Base.deliveries deliveries.size.should == 1 mail = deliveries[0] @@ -222,12 +222,27 @@ describe RequestMailer, "when sending reminders to requesters to classify a resp RequestMailer.alert_new_response_reminders_internal(7, 'new_response_reminder_1') end - it 'should ask for all requests that are awaiting description and whose latest response is older than the number of days given and that are not the holding pen' do - expected_params = {:conditions => [ "awaiting_description = ? and (select created_at from info_request_events where info_request_events.info_request_id = info_requests.id and info_request_events.event_type = 'response' order by created_at desc limit 1) < ? and url_title != 'holding_pen' and user_id is not null", - true, Time.now() - 7.days ], - :include => [ :user ], - :order => "info_requests.id"} - InfoRequest.should_receive(:find).with(:all, expected_params).and_return([]) + it 'should ask for all requests that are awaiting description and whose latest response is older + than the number of days given and that are not the holding pen' do + expected_conditions = [ "awaiting_description = ? + AND (SELECT created_at + FROM info_request_events + WHERE info_request_events.info_request_id = info_requests.id + AND info_request_events.event_type = 'response' + ORDER BY created_at desc LIMIT 1) < ? + AND url_title != 'holding_pen' + AND user_id IS NOT NULL".split(' ').join(' '), + true, Time.now() - 7.days ] + + # compare the query string ignoring any spacing differences + InfoRequest.should_receive(:find) do |all, query_params| + query_string = query_params[:conditions][0] + query_params[:conditions][0] = query_string.split(' ').join(' ') + query_params[:conditions].should == expected_conditions + query_params[:include].should == [ :user ] + query_params[:order].should == 'info_requests.id' + end + send_alerts end |