diff options
41 files changed, 868 insertions, 404 deletions
@@ -0,0 +1,52 @@ +#!/usr/bin/env bash + +# This is an RVM Project .rvmrc file, used to automatically load the ruby +# development environment upon cd'ing into the directory + +# First we specify our desired <ruby>[@<gemset>], the @gemset name is optional, +# Only full ruby name is supported here, for short names use: +# echo "rvm use 1.8.7" > .rvmrc +environment_id="ruby-1.8.7-p370" + +# Uncomment the following lines if you want to verify rvm version per project +# rvmrc_rvm_version="1.14.10 (stable)" # 1.10.1 seams as a safe start +# eval "$(echo ${rvm_version}.${rvmrc_rvm_version} | awk -F. '{print "[[ "$1*65536+$2*256+$3" -ge "$4*65536+$5*256+$6" ]]"}' )" || { +# echo "This .rvmrc file requires at least RVM ${rvmrc_rvm_version}, aborting loading." +# return 1 +# } + +# First we attempt to load the desired environment directly from the environment +# file. This is very fast and efficient compared to running through the entire +# CLI and selector. If you want feedback on which environment was used then +# insert the word 'use' after --create as this triggers verbose mode. +if [[ -d "${rvm_path:-$HOME/.rvm}/environments" + && -s "${rvm_path:-$HOME/.rvm}/environments/$environment_id" ]] +then + \. "${rvm_path:-$HOME/.rvm}/environments/$environment_id" + [[ -s "${rvm_path:-$HOME/.rvm}/hooks/after_use" ]] && + \. "${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. + rvm --create use "$environment_id" || { + echo "Failed to create RVM environment '${environment_id}'." + return 1 + } +fi + +# If you use bundler, this might be useful to you: +# if [[ -s Gemfile ]] && { +# ! builtin command -v bundle >/dev/null || +# builtin command -v bundle | GREP_OPTIONS= \grep $rvm_path/bin/bundle >/dev/null +# } +# then +# printf "%b" "The rubygem 'bundler' is not installed. Installing it now.\n" +# gem install bundler +# fi +# if [[ -s Gemfile ]] && builtin command -v bundle >/dev/null +# then +# bundle install | GREP_OPTIONS= \grep -vE '^Using|Your bundle is complete' +# fi @@ -1,5 +1,5 @@ # Work around bug in Debian Squeeze - see https://github.com/sebbacon/alaveteli/pull/297#issuecomment-4101012 -if File.exist? "/etc/debian_version" and File.open("/etc/debian_version").read.strip =~ /^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') @@ -1,6 +1,6 @@ # Welcome to Alaveteli! -[](http://travis-ci.org/sebbacon/alaveteli) +[](http://travis-ci.org/sebbacon/alaveteli) [](https://gemnasium.com/sebbacon/alaveteli) This is an open source project to create a standard, internationalised platform for making Freedom of Information (FOI) requests in different @@ -22,3 +22,6 @@ There's background information and a more documentation on [our wiki](https://github.com/sebbacon/alaveteli/wiki/Home/), and lots of useful information (including a blog) on [the project website](http://alaveteli.org) + +Looking for the latest stable release? It's on the +[master branch](https://github.com/sebbacon/alaveteli/tree/master). diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index d8fda9c01..08528f8a8 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -36,7 +36,7 @@ class AdminController < ApplicationController # also force a search reindexing (so changed text reflected in search) info_request.reindex_request_events - # and remove from varnsi + # and remove from varnish info_request.purge_in_cache end diff --git a/app/controllers/admin_public_body_controller.rb b/app/controllers/admin_public_body_controller.rb index 7bd794d23..30a43bb81 100644 --- a/app/controllers/admin_public_body_controller.rb +++ b/app/controllers/admin_public_body_controller.rb @@ -142,13 +142,7 @@ class AdminPublicBodyController < AdminController @notes = "" @errors = "" if request.post? - if params['commit'] == 'Dry run' - dry_run_only = true - elsif params['commit'] == 'Upload' - dry_run_only = false - else - raise "internal error, unknown button label" - end + dry_run_only = (params['commit'] == 'Upload' ? false : true) # Read file from params if params[:csv_file] csv_contents = params[:csv_file].read diff --git a/app/controllers/admin_request_controller.rb b/app/controllers/admin_request_controller.rb index ae4bb511a..c5abf8769 100644 --- a/app/controllers/admin_request_controller.rb +++ b/app/controllers/admin_request_controller.rb @@ -28,8 +28,8 @@ class AdminRequestController < AdminController @info_request = InfoRequest.find(params[:id]) # XXX is this *really* the only way to render a template to a # variable, rather than to the response? - vars = OpenStruct.new(:name_to => @info_request.user_name, - :name_from => MySociety::Config.get("CONTACT_NAME", 'Alaveteli'), + vars = OpenStruct.new(:name_to => @info_request.user_name, + :name_from => MySociety::Config.get("CONTACT_NAME", 'Alaveteli'), :info_request => @info_request, :reason => params[:reason], :info_request_url => 'http://' + MySociety::Config.get('DOMAIN') + request_url(@info_request), :site_name => site_name) @@ -81,6 +81,8 @@ class AdminRequestController < AdminController :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 }) + # expire cached files + expire_for_request(@info_request) flash[:notice] = 'Request successfully updated.' redirect_to request_admin_url(@info_request) else @@ -95,7 +97,8 @@ class AdminRequestController < AdminController url_title = @info_request.url_title @info_request.fully_destroy - + # expire cached files + expire_for_request(@info_request) flash[:notice] = "Request #{url_title} has been completely destroyed. Email of user who made request: " + user.email redirect_to admin_url('request/list') end @@ -166,7 +169,8 @@ class AdminRequestController < AdminController @incoming_message.fully_destroy @incoming_message.info_request.log_event("destroy_incoming", { :editor => admin_http_auth_user(), :deleted_incoming_message_id => incoming_message_id }) - + # expire cached files + expire_for_request(@info_request) flash[:notice] = 'Incoming message successfully destroyed.' redirect_to request_admin_url(@info_request) end @@ -174,17 +178,18 @@ class AdminRequestController < AdminController def redeliver_incoming incoming_message = IncomingMessage.find(params[:redeliver_incoming_message_id]) message_ids = params[:url_title].split(",").each {|x| x.strip} + previous_request = incoming_message.info_request destination_request = nil ActiveRecord::Base.transaction do for m in message_ids if m.match(/^[0-9]+$/) destination_request = InfoRequest.find_by_id(m.to_i) else - destination_request = InfoRequest.find_by_url_title(m) + destination_request = InfoRequest.find_by_url_title!(m) end if destination_request.nil? flash[:error] = "Failed to find destination request '" + m + "'" - return redirect_to request_admin_url(incoming_message.info_request) + return redirect_to request_admin_url(previous_request) end raw_email_data = incoming_message.raw_email.data @@ -201,6 +206,8 @@ class AdminRequestController < AdminController flash[:notice] = "Message has been moved to request(s). Showing the last one:" end + # expire cached files + expire_for_request(previous_request) incoming_message.fully_destroy end redirect_to request_admin_url(destination_request) @@ -344,23 +351,29 @@ class AdminRequestController < AdminController explanation = params[:explanation] info_request = InfoRequest.find(params[:id]) info_request.prominence = "requester_only" - + info_request.log_event("hide", { :editor => admin_http_auth_user(), :reason => params[:reason], :subject => subject, :explanation => explanation }) - + info_request.set_described_state(params[:reason]) info_request.save! - ContactMailer.deliver_from_admin_message( - info_request.user, - subject, - params[:explanation] - ) - flash[:notice] = _("Your message to {{recipient_user_name}} has been sent",:recipient_user_name=>CGI.escapeHTML(info_request.user.name)) + if ! info_request.is_external? + ContactMailer.deliver_from_admin_message( + info_request.user, + subject, + params[:explanation] + ) + flash[:notice] = _("Your message to {{recipient_user_name}} has been sent",:recipient_user_name=>CGI.escapeHTML(info_request.user.name)) + else + flash[:notice] = _("This external request has been hidden") + end + # expire cached files + expire_for_request(info_request) redirect_to request_admin_url(info_request) end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 11f21025c..ce18e6ef5 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -134,6 +134,10 @@ class ApplicationController < ActionController::Base # Make sure expiry time for session is set (before_filters are # otherwise missed by this override) session_remember_me + + # Make sure the locale is set correctly too + set_gettext_locale + case exception when ActiveRecord::RecordNotFound, ActionController::UnknownAction, ActionController::RoutingError @status = 404 @@ -157,6 +161,9 @@ class ApplicationController < ActionController::Base # otherwise missed by this override) session_remember_me + # Make sure the locale is set correctly too + set_gettext_locale + # Display default, detailed error for developers original_rescue_action_locally(exception) end @@ -206,13 +213,16 @@ class ApplicationController < ActionController::Base foi_cache_path = File.expand_path(File.join(File.dirname(__FILE__), '../../cache')) return File.join(foi_cache_path, path) end + def foi_fragment_cache_exists?(key_path) return File.exists?(key_path) end + def foi_fragment_cache_read(key_path) logger.info "Reading from fragment cache #{key_path}" return File.read(key_path) end + def foi_fragment_cache_write(key_path, content) FileUtils.mkdir_p(File.dirname(key_path)) logger.info "Writing to fragment cache #{key_path}" @@ -382,8 +392,11 @@ class ApplicationController < ActionController::Base # might fail later if the database has subsequently been reopened. return result end + def get_search_page_from_params - return (params[:page] || "1").to_i + page = (params[:page] || "1").to_i + page = 1 if page < 1 + return page end def perform_search_typeahead(query, model) diff --git a/app/controllers/comment_controller.rb b/app/controllers/comment_controller.rb index d9cd002dd..1552017c2 100644 --- a/app/controllers/comment_controller.rb +++ b/app/controllers/comment_controller.rb @@ -12,7 +12,7 @@ class CommentController < ApplicationController def new if params[:type] == 'request' - @info_request = InfoRequest.find_by_url_title(params[:url_title]) + @info_request = InfoRequest.find_by_url_title!(params[:url_title]) @track_thing = TrackThing.create_track_for_request(@info_request) if params[:comment] @comment = Comment.new(params[:comment].merge({ diff --git a/app/controllers/help_controller.rb b/app/controllers/help_controller.rb index e3b77271e..c7affd57c 100644 --- a/app/controllers/help_controller.rb +++ b/app/controllers/help_controller.rb @@ -15,7 +15,7 @@ class HelpController < ApplicationController def unhappy @info_request = nil if params[:url_title] - @info_request = InfoRequest.find_by_url_title(params[:url_title]) + @info_request = InfoRequest.find_by_url_title!(params[:url_title]) end end diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index 7f42eeb7e..60b33fe28 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -64,10 +64,7 @@ class RequestController < ApplicationController end # Look up by new style text names - @info_request = InfoRequest.find_by_url_title(params[:url_title]) - if @info_request.nil? - raise ActiveRecord::RecordNotFound.new("Request not found") - end + @info_request = InfoRequest.find_by_url_title!(params[:url_title]) set_last_request(@info_request) # Test for whole request being hidden @@ -125,14 +122,10 @@ class RequestController < ApplicationController # Extra info about a request, such as event history def details long_cache - @info_request = InfoRequest.find_by_url_title(params[:url_title]) - if @info_request.nil? - raise ActiveRecord::RecordNotFound.new("Request not found") - else - if !@info_request.user_can_view?(authenticated_user) - render :template => 'request/hidden', :status => 410 # gone - return - end + @info_request = InfoRequest.find_by_url_title!(params[:url_title]) + if !@info_request.user_can_view?(authenticated_user) + render :template => 'request/hidden', :status => 410 # gone + return end @columns = ['id', 'event_type', 'created_at', 'described_state', 'last_described_at', 'calculated_state' ] end @@ -142,7 +135,7 @@ class RequestController < ApplicationController short_cache @per_page = 25 @page = (params[:page] || "1").to_i - @info_request = InfoRequest.find_by_url_title(params[:url_title]) + @info_request = InfoRequest.find_by_url_title!(params[:url_title]) raise ActiveRecord::RecordNotFound.new("Request not found") if @info_request.nil? if !@info_request.user_can_view?(authenticated_user) @@ -313,7 +306,7 @@ class RequestController < ApplicationController # case the list of errors will also contain a more specific error # describing the reason it is invalid. @info_request.errors.delete("outgoing_messages") - + render :action => 'new' return end @@ -431,7 +424,7 @@ class RequestController < ApplicationController }) # Don't give advice on what to do next, as it isn't their request - RequestMailer.deliver_old_unclassified_updated(@info_request) + RequestMailer.deliver_old_unclassified_updated(@info_request) if !@info_request.is_external? if session[:request_game] flash[:notice] = _('Thank you for updating the status of the request \'<a href="{{url}}">{{info_request_title}}</a>\'. There are some more requests below for you to classify.',:info_request_title=>CGI.escapeHTML(@info_request.title), :url=>CGI.escapeHTML(request_url(@info_request))) redirect_to play_url @@ -659,16 +652,21 @@ class RequestController < ApplicationController @info_request = incoming_message.info_request # used by view render :template => 'request/hidden', :status => 410 # gone end + # Is this a completely public request that we can cache attachments for + # to be served up without authentication? + if incoming_message.info_request.all_can_view? + @files_can_be_cached = true + end end def report_request - info_request = InfoRequest.find_by_url_title(params[:url_title]) + info_request = InfoRequest.find_by_url_title!(params[:url_title]) return if !authenticated?( :web => _("To report this FOI request"), :email => _("Then you can report the request '{{title}}'", :title => info_request.title), :email_subject => _("Report an offensive or unsuitable request") ) - + if !info_request.attention_requested info_request.set_described_state('attention_requested', @user) info_request.attention_requested = true # tells us if attention has ever been requested @@ -689,6 +687,7 @@ class RequestController < ApplicationController key = params.merge(:only_path => true) key_path = foi_fragment_cache_path(key) if foi_fragment_cache_exists?(key_path) + logger.info("Reading cache for #{key_path}") raise PermissionDenied.new("Directory listing not allowed") if File.directory?(key_path) cached = foi_fragment_cache_read(key_path) response.content_type = AlaveteliFileTypes.filename_to_mimetype(params[:file_name].join("/")) || 'application/octet-stream' @@ -703,7 +702,10 @@ class RequestController < ApplicationController # various fragment cache functions using Ruby Marshall to write the file # which adds a header, so isnt compatible with images that have been # extracted elsewhere from PDFs) - foi_fragment_cache_write(key_path, response.body) + if @files_can_be_cached == true + logger.info("Writing cache for #{key_path}") + foi_fragment_cache_write(key_path, response.body) + end end end end @@ -784,7 +786,7 @@ class RequestController < ApplicationController def upload_response @locale = self.locale_from_params() PublicBody.with_locale(@locale) do - @info_request = InfoRequest.find_by_url_title(params[:url_title]) + @info_request = InfoRequest.find_by_url_title!(params[:url_title]) @reason_params = { :web => _("To upload a response, you must be logged in using an email address from ") + CGI.escapeHTML(@info_request.public_body.name), @@ -841,10 +843,7 @@ class RequestController < ApplicationController def download_entire_request @locale = self.locale_from_params() PublicBody.with_locale(@locale) do - info_request = InfoRequest.find_by_url_title(params[:url_title]) - if info_request.nil? - raise ActiveRecord::RecordNotFound.new("Request not found") - end + info_request = InfoRequest.find_by_url_title!(params[:url_title]) if authenticated?( :web => _("To download the zip file"), :email => _("Then you can download a zip file of {{info_request_title}}.",:info_request_title=>info_request.title), diff --git a/app/controllers/services_controller.rb b/app/controllers/services_controller.rb index 00c0e61bd..40e0faaf7 100644 --- a/app/controllers/services_controller.rb +++ b/app/controllers/services_controller.rb @@ -3,36 +3,43 @@ require 'open-uri' class ServicesController < ApplicationController + def other_country_message text = "" iso_country_code = MySociety::Config.get('ISO_COUNTRY_CODE').downcase if country_from_ip.downcase != iso_country_code found_country = WorldFOIWebsites.by_code(country_from_ip) found_country_name = !found_country.nil? && found_country[:country_name] - old_locale = FastGettext.locale - FastGettext.locale = FastGettext.best_locale_in(request.env['HTTP_ACCEPT_LANGUAGE']) - if found_country_name - text = _("Hello! You can make Freedom of Information requests within {{country_name}} at {{link_to_website}}", :country_name => found_country_name, :link_to_website => "<a href=\"#{found_country[:url]}\">#{found_country[:name]}</a>") - else - current_country = WorldFOIWebsites.by_code(iso_country_code)[:country_name] - text = _("Hello! We have an <a href=\"/help/alaveteli?country_name=#{CGI.escape(current_country)}\">important message</a> for visitors outside {{country_name}}", :country_name => current_country) + + old_fgt_locale = FastGettext.locale + begin + FastGettext.locale = FastGettext.best_locale_in(request.env['HTTP_ACCEPT_LANGUAGE']) + if found_country_name + text = _("Hello! You can make Freedom of Information requests within {{country_name}} at {{link_to_website}}", :country_name => found_country_name, :link_to_website => "<a href=\"#{found_country[:url]}\">#{found_country[:name]}</a>") + else + current_country = WorldFOIWebsites.by_code(iso_country_code)[:country_name] + text = _("Hello! We have an <a href=\"/help/alaveteli?country_name=#{CGI.escape(current_country)}\">important message</a> for visitors outside {{country_name}}", :country_name => current_country) + end + ensure + FastGettext.locale = old_fgt_locale end - FastGettext.locale = old_locale end if !text.empty? text += ' <span class="close-button">X</span>' end render :text => text, :content_type => "text/plain" # XXX workaround the HTML validation in test suite end + def hidden_user_explanation info_request = InfoRequest.find(params[:info_request_id]) - render :template => "admin_request/hidden_user_explanation", + render :template => "admin_request/hidden_user_explanation", :content_type => "text/plain", :layout => false, - :locals => {:name_to => info_request.user.name, - :name_from => MySociety::Config.get("CONTACT_NAME", 'Alaveteli'), + :locals => {:name_to => info_request.user_name, + :name_from => MySociety::Config.get("CONTACT_NAME", 'Alaveteli'), :info_request => info_request, :reason => params[:reason], :info_request_url => 'http://' + MySociety::Config.get('DOMAIN') + request_url(info_request), :site_name => site_name} end + end diff --git a/app/controllers/track_controller.rb b/app/controllers/track_controller.rb index 07e807451..1a21491b1 100644 --- a/app/controllers/track_controller.rb +++ b/app/controllers/track_controller.rb @@ -15,7 +15,7 @@ class TrackController < ApplicationController # Track all updates to a particular request def track_request - @info_request = InfoRequest.find_by_url_title(params[:url_title]) + @info_request = InfoRequest.find_by_url_title!(params[:url_title]) @track_thing = TrackThing.create_track_for_request(@info_request) return atom_feed_internal if params[:feed] == 'feed' diff --git a/app/helpers/link_to_helper.rb b/app/helpers/link_to_helper.rb index 01332c5ab..16e37277b 100755 --- a/app/helpers/link_to_helper.rb +++ b/app/helpers/link_to_helper.rb @@ -45,9 +45,11 @@ module LinkToHelper def incoming_message_url(incoming_message) return request_url(incoming_message.info_request)+"#incoming-"+incoming_message.id.to_s end + def outgoing_message_url(outgoing_message) return request_url(outgoing_message.info_request)+"#outgoing-"+outgoing_message.id.to_s end + def comment_url(comment) return request_url(comment.info_request)+"#comment-"+comment.id.to_s end @@ -67,21 +69,27 @@ module LinkToHelper def public_body_url(public_body) public_body.url_name.nil? ? '' : show_public_body_url(:url_name => public_body.url_name, :only_path => true) end + def public_body_link_short(public_body) link_to h(public_body.short_or_long_name), public_body_url(public_body) end + def public_body_link(public_body, cls=nil) link_to h(public_body.name), public_body_url(public_body), :class => cls end + def public_body_link_absolute(public_body) # e.g. for in RSS link_to h(public_body.name), main_url(public_body_url(public_body)) end + def public_body_admin_url(public_body) return admin_url('body/show/' + public_body.id.to_s) end + def public_body_both_links(public_body) link_to(h(public_body.name), main_url(public_body_url(public_body))) + " (" + link_to("admin", public_body_admin_url(public_body)) + ")" end + def list_public_bodies_default list_public_bodies_url(:tag => 'all') end @@ -90,20 +98,37 @@ module LinkToHelper def user_url(user) return show_user_url(:url_name => user.url_name, :only_path => true) end + def user_link(user, cls=nil) link_to h(user.name), user_url(user), :class => cls end + def user_link_for_request(request, cls=nil) if request.is_external? - request.external_user_name || _("Anonymous user") + user_name = request.external_user_name || _("Anonymous user") + if !request.external_url.nil? + link_to h(user_name), request.external_url + else + user_name + end else link_to h(request.user.name), user_url(request.user), :class => cls end end + def user_admin_link_for_request(request, external_text=nil, internal_text=nil) + if request.is_external? + text = external_text ? external_text : request.user_name + " (external)" + else + text = internal_text ? internal_text : request.user.name + link_to(h(text), user_admin_url(request.user)) + end + end + def user_link_absolute(user) link_to h(user.name), main_url(user_url(user)) end + def request_user_link_absolute(request) if request.is_external? request.external_user_name || _("Anonymous user") @@ -111,6 +136,7 @@ module LinkToHelper user_link_absolute(request.user) end end + def user_or_you_link(user) if @user && user == @user link_to h("you"), user_url(user) @@ -118,6 +144,7 @@ module LinkToHelper link_to h(user.name), user_url(user) end end + def user_or_you_capital(user) if @user && user == @user return h("You") @@ -125,15 +152,19 @@ module LinkToHelper return h(user.name) end end + def user_or_you_capital_link(user) link_to user_or_you_capital(user), user_url(user) end + def user_admin_url(user) return admin_url('user/show/' + user.id.to_s) end + def user_admin_link(user, name="admin", cls=nil) link_to name, user_admin_url(user), :class => cls end + def user_both_links(user) link_to(h(user.name), main_url(user_url(user))) + " (" + link_to("admin", user_admin_url(user)) + ")" end @@ -199,6 +230,7 @@ module LinkToHelper def about_url return help_general_url(:action => 'about') end + def unhappy_url(info_request = nil) if info_request.nil? return help_general_url(:action => 'unhappy') diff --git a/app/models/exim_log.rb b/app/models/exim_log.rb index 60faa7f0b..82000efa1 100644 --- a/app/models/exim_log.rb +++ b/app/models/exim_log.rb @@ -94,7 +94,7 @@ class EximLog < ActiveRecord::Base # Get all requests sent for from 2 to 10 days ago. The 2 day gap is # because we load exim log lines via cron at best an hour after they # are made) - irs = InfoRequest.find(:all, :conditions => [ "created_at < ? and created_at > ?", Time.now() - 2.day, Time.now() - 10.days ] ) + irs = InfoRequest.find(:all, :conditions => [ "created_at < ? and created_at > ? and user_id is not null", Time.now() - 2.day, Time.now() - 10.days ] ) # Go through each request and check it ok = true diff --git a/app/models/foi_attachment.rb b/app/models/foi_attachment.rb index 9bbf0988f..a40898aef 100644 --- a/app/models/foi_attachment.rb +++ b/app/models/foi_attachment.rb @@ -1,3 +1,5 @@ +# encoding: UTF-8 + # == Schema Information # Schema version: 114 # @@ -14,8 +16,6 @@ # hexdigest :string(32) # -# encoding: UTF-8 - # models/foi_attachment.rb: # An attachment to an email (IncomingMessage) # @@ -315,14 +315,21 @@ class FoiAttachment < ActiveRecord::Base tempfile.print self.body tempfile.flush + html = nil if self.content_type == 'application/pdf' - html = AlaveteliExternalCommand.run("pdftohtml", "-nodrm", "-zoom", "1.0", "-stdout", "-enc", "UTF-8", "-noframes", tempfile.path) + # We set a timeout here, because pdftohtml can spiral out of control + # on some PDF files and we don’t want to crash the whole server. + html = AlaveteliExternalCommand.run("pdftohtml", "-nodrm", "-zoom", "1.0", "-stdout", "-enc", "UTF-8", "-noframes", tempfile.path, :timeout => 30) elsif self.content_type == 'application/rtf' - html = AlaveteliExternalCommand.run("unrtf", "--html", tempfile.path) - elsif self.has_google_docs_viewer? - html = '' # force error and using Google docs viewer - else - raise "No HTML conversion available for type " + self.content_type + html = AlaveteliExternalCommand.run("unrtf", "--html", tempfile.path, :timeout => 120) + end + + if html.nil? + if self.has_google_docs_viewer? + html = '' # force error and using Google docs viewer + else + raise "No HTML conversion available for type " + self.content_type + end end tempfile.close diff --git a/app/models/info_request.rb b/app/models/info_request.rb index dfaa524b2..6f472c290 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -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 @@ -246,7 +246,9 @@ public # For request with same title as others, add on arbitary numeric identifier unique_url_title = url_title suffix_num = 2 # as there's already one without numeric suffix - while not InfoRequest.find_by_url_title(unique_url_title, :conditions => self.id.nil? ? nil : ["id <> ?", self.id] ).nil? + while not InfoRequest.find_by_url_title(unique_url_title, + :conditions => self.id.nil? ? nil : ["id <> ?", self.id] + ).nil? unique_url_title = url_title + "_" + suffix_num.to_s suffix_num = suffix_num + 1 end @@ -456,7 +458,7 @@ public if !allow if self.handle_rejected_responses == 'bounce' - RequestMailer.deliver_stopped_responses(self, email, raw_email_data) + RequestMailer.deliver_stopped_responses(self, email, raw_email_data) if !is_external? elsif self.handle_rejected_responses == 'holding_pen' InfoRequest.holding_pen_request.receive(email, raw_email_data, false, reason) elsif self.handle_rejected_responses == 'blackhole' @@ -566,7 +568,10 @@ public self.calculate_event_states if self.requires_admin? - RequestMailer.deliver_requires_admin(self, set_by) + # Check there is someone to send the message "from" + if !set_by.nil? || !self.user.nil? + RequestMailer.deliver_requires_admin(self, set_by) + end end end @@ -942,7 +947,7 @@ public 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'", + :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] @@ -960,6 +965,7 @@ public end def is_old_unclassified? + return false if is_external? return false if !awaiting_description return false if url_title == 'holding_pen' last_response_event = get_last_response_event @@ -1036,6 +1042,12 @@ public return true end + # Is this request visible to everyone? + def all_can_view? + return true if ['normal', 'backpage'].include?(self.prominence) + return false + end + def indexed_by_search? if self.prominence == 'backpage' || self.prominence == 'hidden' || self.prominence == 'requester_only' return false diff --git a/app/models/request_mailer.rb b/app/models/request_mailer.rb index 03d26f237..ba9285fc6 100644 --- a/app/models/request_mailer.rb +++ b/app/models/request_mailer.rb @@ -28,17 +28,17 @@ class RequestMailer < ApplicationMailer :filename => attachment_name end end - + # Used when a response is uploaded using the API def external_response(info_request, body, sent_at, attachments) @from = blackhole_email @recipients = info_request.incoming_name_and_email @body = { :body => body } - + # ActionMailer only works properly when the time is in the local timezone: # see https://rails.lighthouseapp.com/projects/8994/tickets/3113-actionmailer-only-works-correctly-with-sent_on-times-that-are-in-the-local-time-zone @sent_on = sent_at.dup.localtime - + attachments.each do |attachment_hash| attachment attachment_hash end @@ -392,6 +392,7 @@ class RequestMailer < ApplicationMailer ) for info_request in info_requests + next if info_request.is_external? # Count number of new comments to alert on earliest_unalerted_comment_event = nil last_comment_event = nil diff --git a/app/views/admin_public_body/import_csv.rhtml b/app/views/admin_public_body/import_csv.rhtml index 4eb83cc7e..4a03d0665 100644 --- a/app/views/admin_public_body/import_csv.rhtml +++ b/app/views/admin_public_body/import_csv.rhtml @@ -44,7 +44,7 @@ <blockquote> <p> - #id,name,request_email,name.es,tag_string<br/> + #id,name,request_email,name.es,tag_string<br/> 1,An Authority,a@example.com,Un organismo,a_tag another_tag<br/> 2,Another One,another@example.com,Otro organismo,a_tag<br/> <p> diff --git a/app/views/admin_request/edit.rhtml b/app/views/admin_request/edit.rhtml index 4026ee712..808028b47 100644 --- a/app/views/admin_request/edit.rhtml +++ b/app/views/admin_request/edit.rhtml @@ -7,21 +7,21 @@ <p><label for="info_request_title"><strong>Title</strong></label> (warning: editing this will break URLs right now)<br/> <%= text_field 'info_request', 'title', :size => 50 %></p> - <p><label for="info_request_prominence"><strong>Prominence</strong></label> + <p><label for="info_request_prominence"><strong>Prominence</strong></label> <%= select( 'info_request', "prominence", [ "normal", "backpage", "requester_only", "hidden" ]) %> (backpage means hidden from lists/search; hidden means completely hidden; super users can see anything) </p> <p> - <label for="info_request_allow_new_responses_from"><strong>Allow new responses</strong> from</label> + <label for="info_request_allow_new_responses_from"><strong>Allow new responses</strong> from</label> <%= select( 'info_request', "allow_new_responses_from", [ "anybody", "authority_only", "nobody" ] ) %>; - <label for="info_request_handle_rejected_responses"><strong>Handle rejected responses</strong> with</label> + <label for="info_request_handle_rejected_responses"><strong>Handle rejected responses</strong> with</label> <%= select( 'info_request', "handle_rejected_responses", [ "bounce", "holding_pen", "blackhole" ] ) %> <br> ('authority_only' means email From: domain of authority request email or any domain that has previously sent a response; 'nobody' also stops requester making followups; take care when using 'blackhole' which just drops mail) </p> - <p><label for="info_request_described_state"><strong>Described state</strong></label> + <p><label for="info_request_described_state"><strong>Described state</strong></label> <%= select( 'info_request', "described_state", InfoRequest.enumerate_states ) %>; <label for="info_request_awaiting_description"><strong>Awaiting description</strong></label> <%= select('info_request', "awaiting_description", [["Yes - needs state updating",true],["No - state is up to date",false]]) %> @@ -49,7 +49,7 @@ <% form_tag '../destroy/' + @info_request.id.to_s do %> <p> - <strong>This is permanent and irreversible!</strong> <%= submit_tag 'Destory request entirely' %> + <strong>This is permanent and irreversible!</strong> <%= submit_tag 'Destroy request entirely' %> <br>Use it mainly if someone posts private information, e.g. made a Data Protection request. It destroys all responses and tracks as well. </p> diff --git a/app/views/request/_correspondence.rhtml b/app/views/request/_correspondence.rhtml index 36257991b..bcfc93657 100644 --- a/app/views/request/_correspondence.rhtml +++ b/app/views/request/_correspondence.rhtml @@ -40,7 +40,7 @@ elsif [ 'sent', 'followup_sent' ].include?(info_request_event.event_type) <%= render :partial => 'bubble', :locals => { :body => outgoing_message.get_body_for_html_display(), :attachments => nil } %> <p class="event_actions"> - <% if outgoing_message.status == 'ready' %> + <% if outgoing_message.status == 'ready' && !@info_request.is_external? %> <strong>Warning:</strong> This message has <strong>not yet been sent</strong> for an unknown reason. <% end %> diff --git a/app/views/request/show.rhtml b/app/views/request/show.rhtml index a7760ab19..34c7a98b0 100644 --- a/app/views/request/show.rhtml +++ b/app/views/request/show.rhtml @@ -35,57 +35,57 @@ <p class="subtitle"> <% if !@user.nil? && @user.admin_page_links? %> - <%= _('{{user}} (<a href="{{user_admin_url}}">admin</a>) made this {{law_used_full}} request (<a href="{{request_admin_url}}">admin</a>) to {{public_body_link}} (<a href="{{public_body_admin_url}}">admin</a>)', - :user => user_link(@info_request.user), + <%= _('{{user}} ({{user_admin_link}}) made this {{law_used_full}} request (<a href="{{request_admin_url}}">admin</a>) to {{public_body_link}} (<a href="{{public_body_admin_url}}">admin</a>)', + :user => @info_request.is_external? ? (@info_request.user_name || _('An anonymous user')) : user_link(@info_request.user), :law_used_full => h(@info_request.law_used_full), - :user_admin_url => user_admin_url(@info_request.user), + :user_admin_link => user_admin_link_for_request(@info_request, _('external'), _('admin')), :request_admin_url => request_admin_url(@info_request), :public_body_link => public_body_link(@info_request.public_body), :public_body_admin_url => public_body_admin_url(@info_request.public_body)) %> <% else %> - <%= _('{{user}} made this {{law_used_full}} request',:user=>@info_request.user.nil? ? @info_request.user_name : user_link(@info_request.user), :law_used_full=>h(@info_request.law_used_full)) %> - <%= _('to {{public_body}}',:public_body=>public_body_link(@info_request.public_body)) %> + <%= _('{{user}} made this {{law_used_full}} request',:user=>@info_request.is_external? ? (@info_request.user_name || _('An anonymous user')) : user_link(@info_request.user), :law_used_full=>h(@info_request.law_used_full)) %> + <%= _('to {{public_body}}',:public_body=>public_body_link(@info_request.public_body)) %> <% end %> </p> <p id="request_status" class="request_icon_line icon_<%= @info_request.calculate_status %>"> <% if @info_request.awaiting_description %> <% if @is_owning_user %> - <%= _('Please <strong>answer the question above</strong> so we know whether the ')%> + <%= _('Please <strong>answer the question above</strong> so we know whether the ')%> <%= MySociety::Format.fancy_pluralize(@new_responses_count, 'recent response contains', 'recent responses contain') %> <%= _('useful information.') %> <% else %> <%= _('This request has an <strong>unknown status</strong>.') %> <% if @old_unclassified %> - <%= _('We\'re waiting for someone to read') %> + <%= _('We\'re waiting for someone to read') %> <%= MySociety::Format.fancy_pluralize(@new_responses_count, 'a recent response', 'recent responses') %> <%= _('and update the status accordingly. Perhaps <strong>you</strong> might like to help out by doing that?') %> <% else %> <%= _('We\'re waiting for') %> - <%= user_link_for_request(@info_request) %> <%= _('to read') %> - <%= MySociety::Format.fancy_pluralize(@new_responses_count, 'a recent response', 'recent responses') %> + <%= user_link_for_request(@info_request) %> <%= _('to read') %> + <%= MySociety::Format.fancy_pluralize(@new_responses_count, 'a recent response', 'recent responses') %> <%= _('and update the status.') %> <% end %> <% end %> <% elsif @status == 'waiting_response' %> - <%= _('Currently <strong>waiting for a response</strong> from {{public_body_link}}, they must respond promptly and', :public_body_link=> public_body_link(@info_request.public_body)) %> + <%= _('Currently <strong>waiting for a response</strong> from {{public_body_link}}, they must respond promptly and', :public_body_link=> public_body_link(@info_request.public_body)) %> <% if @info_request.public_body.is_school? %> <%= _('in term time') %> <% else %> - <%= _('normally') %> + <%= _('normally') %> <% end %> <%= _('no later than') %> <strong><%= simple_date(@info_request.date_response_required_by) %></strong> (<%= link_to _("details"), "/help/requesting#quickly_response" %>). <% elsif @status == 'waiting_response_overdue' %> <%= _('Response to this request is <strong>delayed</strong>.') %> - <%= _('By law, {{public_body_link}} should normally have responded <strong>promptly</strong> and',:public_body_link=>public_body_link(@info_request.public_body)) %> + <%= _('By law, {{public_body_link}} should normally have responded <strong>promptly</strong> and',:public_body_link=>public_body_link(@info_request.public_body)) %> <% if @info_request.public_body.is_school? %> - <%= _('in term time') %> + <%= _('in term time') %> <% end %> <%= _('by') %> <strong><%= simple_date(@info_request.date_response_required_by) %></strong> - (<%= _('<a href="%s">details</a>') % [help_requesting_path + '#quickly_response'] %>) + (<%= _('<a href="%s">details</a>') % [help_requesting_path + '#quickly_response'] %>) <% elsif @status == 'waiting_response_very_overdue' %> - <%= _('Response to this request is <strong>long overdue</strong>.') %> - <%= _('By law, under all circumstances, {{public_body_link}} should have responded by now',:public_body_link => public_body_link(@info_request.public_body)) %> + <%= _('Response to this request is <strong>long overdue</strong>.') %> + <%= _('By law, under all circumstances, {{public_body_link}} should have responded by now',:public_body_link => public_body_link(@info_request.public_body)) %> (<%= _('<a href="%s">details</a>') % [help_requesting_path + '#quickly_response'] %>). <%= _('You can <strong>complain</strong> by') %> <%= link_to _("requesting an internal review"), show_response_no_followup_url(:id => @info_request.id, :incoming_message_id => nil) + "?internal_review=1#followup" %>. @@ -99,25 +99,25 @@ <%= _('The request was <strong>partially successful</strong>.') %> <% elsif @status == 'waiting_clarification' %> <% if @is_owning_user %> - <%=h @info_request.public_body.name %> <%= _('is <strong>waiting for your clarification</strong>.') %> + <%=h @info_request.public_body.name %> <%= _('is <strong>waiting for your clarification</strong>.') %> <%= _('Please') %> <%= link_to _("send a follow up message"), respond_to_last_url(@info_request) + '#followup' %>. <% else %> - <%= _('The request is <strong>waiting for clarification</strong>.') %> - <%= _('If you are {{user_link}}, please',:user_link=>user_link_for_request(@info_request)) %> + <%= _('The request is <strong>waiting for clarification</strong>.') %> + <%= _('If you are {{user_link}}, please',:user_link=>user_link_for_request(@info_request)) %> <%= link_to _("sign in"), signin_url(:r => request.request_uri) %> <%= _('to send a follow up message.') %> <% end %> <% elsif @status == 'gone_postal' %> <%= _('The authority would like to / has <strong>responded by post</strong> to this request.') %> <% elsif @status == 'internal_review' %> - <%= _('Waiting for an <strong>internal review</strong> by {{public_body_link}} of their handling of this request.',:public_body_link=>public_body_link(@info_request.public_body)) %> + <%= _('Waiting for an <strong>internal review</strong> by {{public_body_link}} of their handling of this request.',:public_body_link=>public_body_link(@info_request.public_body)) %> <% elsif @status == 'error_message' %> <%= _('There was a <strong>delivery error</strong> or similar, which needs fixing by the {{site_name}} team.', :site_name=>site_name) %> <% elsif @status == 'requires_admin' %> <%= _('This request has had an unusual response, and <strong>requires attention</strong> from the {{site_name}} team.', :site_name=>site_name) %> <% elsif @status == 'user_withdrawn' %> - <%= _('This request has been <strong>withdrawn</strong> by the person who made it. - There may be an explanation in the correspondence below.') %> + <%= _('This request has been <strong>withdrawn</strong> by the person who made it. + There may be an explanation in the correspondence below.') %> <% elsif @status == 'attention_requested' %> <%= _('This request has been <strong>reported</strong> as needing administrator attention (perhaps because it is vexatious, or a request for personal information)') %> <% elsif @status == 'vexatious' %> diff --git a/app/views/request/simple_correspondence.rhtml b/app/views/request/simple_correspondence.rhtml index 45b90b84b..bcbc795e7 100644 --- a/app/views/request/simple_correspondence.rhtml +++ b/app/views/request/simple_correspondence.rhtml @@ -4,16 +4,16 @@ <% incoming_message = nil if info_request_event.visible - if !info_request_event.nil? && info_request_event.event_type == 'response' - incoming_message = info_request_event.incoming_message - end + if !info_request_event.nil? && info_request_event.event_type == 'response' + incoming_message = info_request_event.incoming_message + end if not incoming_message.nil? if !incoming_message.safe_mail_from.nil? && incoming_message.safe_mail_from.strip != @info_request.public_body.name.strip %> <%= _('From:') %> <%= incoming_message.safe_mail_from %><% end if incoming_message.safe_mail_from.nil? || (incoming_message.mail_from_domain == @info_request.public_body.request_email_domain) %>, <%= @info_request.public_body.name %><% end %> -<%= _('To:') %> <%= @info_request.user.name %> +<%= _('To:') %> <% if @info_request.user_name %><%= @info_request.user_name %><% else %><%= "[#{_('An anonymous user')}]"%><% end %> <%= _('Date:') %> <%= simple_date(incoming_message.sent_at) %> <%= incoming_message.get_body_for_quoting %> @@ -24,7 +24,7 @@ elsif [ 'sent', 'followup_sent' ].include?(info_request_event.event_type) outgoing_message = info_request_event.outgoing_message %> -<%= _('From:') %> <%= @info_request.user.name %> +<%= _('From:') %> <% if @info_request.user_name %><%= @info_request.user_name %><% else %><%= "[#{_('An anonymous user')}]"%><% end %> <%= _('To:') %> <%= @info_request.public_body.name %> <%= _('Date:') %> <%= simple_date(info_request_event.created_at) %> <% @@ -36,7 +36,7 @@ elsif [ 'sent', 'followup_sent' ].include?(info_request_event.event_type) <%= _('Date:') %> <%= simple_date(info_request_event.created_at) %> Sent <% if info_request_event.outgoing_message.message_type == 'initial_request' %> request <% elsif info_request_event.outgoing_message.message_type == 'followup' %> a follow up <% else %> <% raise "unknown message_type" %><% end %> to <%= public_body_link(@info_request.public_body) %> again<% if not info_request_event.same_email_as_previous_send? %>, using a new contact address<% end %>. -<% elsif info_request_event.event_type == 'comment' +<% elsif info_request_event.event_type == 'comment' comment = info_request_event.comment %> <%= _("{{username}} left an annotation:", :username =>comment.user.name) %> (<%= simple_date(comment.created_at || Time.now) %>) diff --git a/commonlib b/commonlib -Subproject 9e1d29721b9dba232c251ef4b8b79f8505422de +Subproject 131375c752c02aa20f1644240fe69720275da42 diff --git a/config/general.yml-example b/config/general.yml-example index 019eb7ada..33e3ad5bf 100644 --- a/config/general.yml-example +++ b/config/general.yml-example @@ -37,7 +37,7 @@ FRONTPAGE_PUBLICBODY_EXAMPLES: 'tgq' # URLs of themes to download and use (when running rails-post-deploy # script). Earlier in the list means the templates have a higher # priority. -THEME_URLS: +THEME_URLS: - 'git://github.com/sebbacon/adminbootstraptheme.git' - 'git://github.com/sebbacon/alavetelitheme.git' diff --git a/config/httpd.conf b/config/httpd.conf index 3bbe50fb3..36f567964 100644 --- a/config/httpd.conf +++ b/config/httpd.conf @@ -5,7 +5,7 @@ # # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org -# +# # $Id: httpd.conf,v 1.31 2009-09-02 13:05:48 matthew Exp $ # This is needed for the PHP spell checker @@ -25,13 +25,26 @@ RewriteEngine On # it. RewriteCond %{LA-U:REMOTE_USER} (.+) RewriteRule . - [E=RU:%1] -RequestHeader add X-Forwarded-User %{RU}e +RequestHeader add X-Forwarded-User %{RU}e # Old /files URL to new subdomain (as can't use Alias with passenger, # so we do it on its own domain). This is for custom admin upload # files for archiving. RewriteRule /files/(.+) http://files.whatdotheyknow.com/$1 +# Serve attachments directly from the cache, if possible. +# +# The file names are URL-encoded on disk, and sharded by the first +# three digits of the request id, which is why this is as complicated +# as it is. The RewriteMap directive makes the URL-escaping function +# available to use in the other directives. +# +# The condition means that the rule will fire only if the cached +# file exists. +RewriteMap escape int:escape +RewriteCond %{DOCUMENT_ROOT}/views_cache/request/$2/$1/${escape:$3} -f +RewriteRule ^/request/((\d{1,3})\d*)/(response/\d+/attach/\d+/.+) /views_cache/request/$2/$1/${escape:$3} [L] + <IfModule mod_passenger.c> # Set this to something like 100 if you have memory leak issues PassengerMaxRequests 20 diff --git a/config/initializers/single_quote_escape_workaround.rb b/config/initializers/single_quote_escape_workaround.rb new file mode 100644 index 000000000..2e713b982 --- /dev/null +++ b/config/initializers/single_quote_escape_workaround.rb @@ -0,0 +1,31 @@ +class ERB
+ module Util
+
+ if "html_safe exists".respond_to?(:html_safe)
+ def html_escape(s)
+ s = s.to_s
+ if s.html_safe?
+ s
+ else
+ Rack::Utils.escape_html(s).html_safe
+ end
+ end
+ else
+ def html_escape(s)
+ s = s.to_s
+ Rack::Utils.escape_html(s).html_safe
+ end
+ end
+
+ remove_method :h
+ alias h html_escape
+
+ class << self
+ remove_method :html_escape
+ remove_method :h
+ end
+
+ module_function :html_escape
+ module_function :h
+ end
+end
diff --git a/config/routes.rb b/config/routes.rb index a9c2c889a..34232b55b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -80,7 +80,7 @@ ActionController::Routing::Routes.draw do |map| # Use /profile for things to do with the currently signed in user. # Use /user/XXXX for things that anyone can see about that user. - # Note that /profile isn't indexe by search (see robots.txt) + # Note that /profile isn't indexed by search (see robots.txt) map.with_options :controller => 'user' do |user| user.signin '/profile/sign_in', :action => 'signin' user.signup '/profile/sign_up', :action => 'signup' diff --git a/db/migrate/113_add_external_fields_to_info_requests.rb b/db/migrate/113_add_external_fields_to_info_requests.rb index 3aea57766..1ad7634ff 100644 --- a/db/migrate/113_add_external_fields_to_info_requests.rb +++ b/db/migrate/113_add_external_fields_to_info_requests.rb @@ -4,6 +4,7 @@ class AddExternalFieldsToInfoRequests < ActiveRecord::Migration add_column :info_requests, :external_user_name, :string, :null => true add_column :info_requests, :external_url, :string, :null => true + # NB This is corrected in 20120822145640 if ActiveRecord::Base.connection.adapter_name == "PostgreSQL" execute "ALTER TABLE info_requests ADD CONSTRAINT info_requests_external_ck CHECK ( (user_id is null) = (external_url is not null) and (external_user_name is not null or external_url is null) )" end diff --git a/db/migrate/20120822145640_correct_external_request_constraint.rb b/db/migrate/20120822145640_correct_external_request_constraint.rb new file mode 100644 index 000000000..6902f3cb2 --- /dev/null +++ b/db/migrate/20120822145640_correct_external_request_constraint.rb @@ -0,0 +1,15 @@ +class CorrectExternalRequestConstraint < ActiveRecord::Migration + def self.up + if ActiveRecord::Base.connection.adapter_name == "PostgreSQL" + execute "ALTER TABLE info_requests DROP CONSTRAINT info_requests_external_ck" + execute "ALTER TABLE info_requests ADD CONSTRAINT info_requests_external_ck CHECK ( (user_id is null) = (external_url is not null) and (external_url is not null or external_user_name is null) )" + end + end + + def self.down + if ActiveRecord::Base.connection.adapter_name == "PostgreSQL" + execute "ALTER TABLE info_requests DROP CONSTRAINT info_requests_external_ck" + execute "ALTER TABLE info_requests ADD CONSTRAINT info_requests_external_ck CHECK ( (user_id is null) = (external_url is not null) and (external_user_name is not null or external_url is null) )" + end + end +end diff --git a/doc/INSTALL.md b/doc/INSTALL.md index b71b89676..588f8e4fe 100644 --- a/doc/INSTALL.md +++ b/doc/INSTALL.md @@ -19,12 +19,11 @@ Next, get hold of the Alaveteli source code from github: git clone https://github.com/sebbacon/alaveteli.git cd alaveteli -This will get the current stable release from the master branch (which -always contains the latest release). If you are a developer and want -to add or try new features, you might want to swap to the development -branch: +This will get the development branch, which has the latest (possibly +buggy) code. If you don't want to add or try new features, swap to the +master branch (which always contains the latest stable release): - git checkout develop + git checkout master # Install system dependencies diff --git a/doc/TRANSLATE.md b/doc/TRANSLATE.md index 97ef39954..543c16cd9 100644 --- a/doc/TRANSLATE.md +++ b/doc/TRANSLATE.md @@ -66,7 +66,7 @@ must: * this updates the PO template, but also merges it with the individual PO files, marking strings that have only changed slightly as "fuzzy" - * reupload (`tx push -s -t`) the POT and PO files to Transifex to the + * reupload (`tx push -s -t`) the POT and PO files to Transifex from the current release branch * The point of uploading the PO files is that Transifex converts the "fuzzy" suggestions from Transifex into diff --git a/lib/quiet_opener.rb b/lib/quiet_opener.rb index 8cedad250..bde645d0b 100644 --- a/lib/quiet_opener.rb +++ b/lib/quiet_opener.rb @@ -5,7 +5,7 @@ require 'net/http/local' def quietly_try_to_open(url) begin result = open(url).read.strip - rescue OpenURI::HTTPError, SocketError, Errno::ETIMEDOUT, Errno::ECONNREFUSED, Errno::EHOSTUNREACH + rescue OpenURI::HTTPError, SocketError, Errno::ETIMEDOUT, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, Errno::ECONNRESET Rails.logger.warn("Unable to open third-party URL #{url}") result = "" end @@ -24,7 +24,7 @@ def quietly_try_to_purge(host, url) result_body = response.body } end - rescue OpenURI::HTTPError, SocketError, Errno::ETIMEDOUT, Errno::ECONNREFUSED, Errno::EHOSTUNREACH + rescue OpenURI::HTTPError, SocketError, Errno::ETIMEDOUT, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, Errno::ECONNRESET Rails.logger.warn("PURGE: Unable to reach host #{host}") end if result == "200" diff --git a/lib/tasks/temp.rake b/lib/tasks/temp.rake index ce04c7ddd..9decc13db 100644 --- a/lib/tasks/temp.rake +++ b/lib/tasks/temp.rake @@ -1,15 +1,44 @@ namespace :temp do - 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| - if post_redirect.post_params && post_redirect.post_params[:signchangeemail] && post_redirect.post_params[:signchangeemail][:password] - params = post_redirect.post_params - params[:signchangeemail].delete(:password) - post_redirect.post_params = params - post_redirect.save! - 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| + if post_redirect.post_params && post_redirect.post_params[:signchangeemail] && post_redirect.post_params[:signchangeemail][:password] + params = post_redirect.post_params + params[:signchangeemail].delete(:password) + post_redirect.post_params = params + post_redirect.save! + end + end + end + + desc 'Remove file caches for requests that are not publicly visible or have been destroyed' + task :remove_obsolete_info_request_caches => :environment do + dryrun = ENV['DRYRUN'] == '0' ? false : true + verbose = ENV['VERBOSE'] == '0' ? false : true + if dryrun + puts "Running in dryrun mode" + end + request_cache_path = File.join(Rails.root, 'cache', 'views', 'request', '*', '*') + Dir.glob(request_cache_path) do |request_subdir| + info_request_id = File.basename(request_subdir) + puts "Looking for InfoRequest with id #{info_request_id}" if verbose + begin + info_request = InfoRequest.find(info_request_id) + puts "Got InfoRequest #{info_request_id}" if verbose + if ! info_request.all_can_view? + puts "Deleting cache at #{request_subdir} for hidden/requester_only InfoRequest #{info_request_id}" + if ! dryrun + FileUtils.rm_rf(request_subdir) + end + end + rescue ActiveRecord::RecordNotFound + puts "Deleting cache at #{request_subdir} for deleted InfoRequest #{info_request_id}" + if ! dryrun + FileUtils.rm_rf(request_subdir) + end + end + end end - end end diff --git a/public/views_cache b/public/views_cache new file mode 120000 index 000000000..ea65934d2 --- /dev/null +++ b/public/views_cache @@ -0,0 +1 @@ +../cache/views
\ No newline at end of file diff --git a/script/load-exim-logs b/script/load-exim-logs index 5ca0c66f8..00b6b9825 100755 --- a/script/load-exim-logs +++ b/script/load-exim-logs @@ -5,7 +5,10 @@ LOC=`dirname "$0"` # Specific file if specified if [ x$1 != x ] then - f=`abspath "$1"` + case "$1" in + /*) f=$1 ;; + *) f=$(pwd)/$1 ;; + esac cd "$LOC" bundle exec ./runner 'EximLog.load_file("'$f'")' exit diff --git a/spec/controllers/admin_request_controller_spec.rb b/spec/controllers/admin_request_controller_spec.rb index b0468822a..252818452 100644 --- a/spec/controllers/admin_request_controller_spec.rb +++ b/spec/controllers/admin_request_controller_spec.rb @@ -27,12 +27,31 @@ describe AdminRequestController, "when administering requests" do it "saves edits to a request" do info_requests(:fancy_dog_request).title.should == "Why do you have & such a fancy dog?" - post :update, { :id => info_requests(:fancy_dog_request), :info_request => { :title => "Renamed", :prominence => "normal", :described_state => "waiting_response", :awaiting_description => false, :allow_new_responses_from => 'anybody', :handle_rejected_responses => 'bounce' } } + post :update, { :id => info_requests(:fancy_dog_request), + :info_request => { :title => "Renamed", + :prominence => "normal", + :described_state => "waiting_response", + :awaiting_description => false, + :allow_new_responses_from => 'anybody', + :handle_rejected_responses => 'bounce' } } response.flash[:notice].should include('successful') ir = InfoRequest.find(info_requests(:fancy_dog_request).id) ir.title.should == "Renamed" end + it 'expires the request cache when saving edits to it' do + info_request = info_requests(:fancy_dog_request) + @controller.should_receive(:expire_for_request).with(info_request) + post :update, { :id => info_request, + :info_request => { :title => "Renamed", + :prominence => "normal", + :described_state => "waiting_response", + :awaiting_description => false, + :allow_new_responses_from => 'anybody', + :handle_rejected_responses => 'bounce' } } + + end + it "edits an outgoing message" do get :edit_outgoing, :id => outgoing_messages(:useless_outgoing_message) end @@ -45,6 +64,16 @@ describe AdminRequestController, "when administering requests" do ir.body.should include("delicious cat") end + describe 'when fully destroying a request' do + + it 'expires the file cache for that request' do + info_request = info_requests(:badger_request) + @controller.should_receive(:expire_for_request).with(info_request) + get :fully_destroy, { :id => info_request } + end + + end + end describe AdminRequestController, "when administering the holding pen" do @@ -80,12 +109,13 @@ describe AdminRequestController, "when administering the holding pen" do InfoRequest.holding_pen_request.incoming_messages.length.should == 1 new_im = InfoRequest.holding_pen_request.incoming_messages[0] ir.incoming_messages.length.should == 1 - post :redeliver_incoming, :redeliver_incoming_message_id => new_im.id, :url_title => ir.url_title + post :redeliver_incoming, :redeliver_incoming_message_id => new_im.id, :url_title => ir.url_title ir = InfoRequest.find_by_url_title(ir.url_title) ir.incoming_messages.length.should == 2 response.should redirect_to(:controller=>'admin_request', :action=>'show', :id=>101) InfoRequest.holding_pen_request.incoming_messages.length.should == 0 end + it "allows redelivery to more than one request" do ir1 = info_requests(:fancy_dog_request) ir1.allow_new_responses_from = 'nobody' @@ -99,7 +129,7 @@ describe AdminRequestController, "when administering the holding pen" do InfoRequest.holding_pen_request.incoming_messages.length.should == 1 new_im = InfoRequest.holding_pen_request.incoming_messages[0] - post :redeliver_incoming, :redeliver_incoming_message_id => new_im.id, :url_title => "#{ir1.url_title},#{ir2.url_title}" + post :redeliver_incoming, :redeliver_incoming_message_id => new_im.id, :url_title => "#{ir1.url_title},#{ir2.url_title}" ir1.reload ir1.incoming_messages.length.should == 2 ir2.reload @@ -108,6 +138,15 @@ describe AdminRequestController, "when administering the holding pen" do InfoRequest.holding_pen_request.incoming_messages.length.should == 0 end + it 'expires the file cache for the previous request' do + current_info_request = info_requests(:fancy_dog_request) + destination_info_request = info_requests(:naughty_chicken_request) + incoming_message = incoming_messages(:useless_incoming_message) + @controller.should_receive(:expire_for_request).with(current_info_request) + post :redeliver_incoming, :redeliver_incoming_message_id => incoming_message.id, + :url_title => destination_info_request.url_title + end + it "guesses a misdirected request" do ir = info_requests(:fancy_dog_request) ir.handle_rejected_responses = 'holding_pen' @@ -124,11 +163,31 @@ describe AdminRequestController, "when administering the holding pen" do assigns[:info_requests][0].should == ir end - it "destroys an incoming message" do - im = incoming_messages(:useless_incoming_message) - raw_email = im.raw_email.filepath - post :destroy_incoming, :incoming_message_id => im.id - assert_equal File.exists?(raw_email), false + describe 'when destroying an incoming message' do + + before do + @im = incoming_messages(:useless_incoming_message) + @controller.stub!(:expire_for_request) + end + + it "destroys the raw email file" do + raw_email = @im.raw_email.filepath + assert_equal File.exists?(raw_email), true + post :destroy_incoming, :incoming_message_id => @im.id + assert_equal File.exists?(raw_email), false + end + + it 'asks the incoming message to fully destroy itself' do + IncomingMessage.stub!(:find).and_return(@im) + @im.should_receive(:fully_destroy) + post :destroy_incoming, :incoming_message_id => @im.id + end + + it 'expires the file cache for the associated info_request' do + @controller.should_receive(:expire_for_request).with(@im.info_request) + post :destroy_incoming, :incoming_message_id => @im.id + end + end it "shows a suitable default 'your email has been hidden' message" do @@ -141,16 +200,76 @@ describe AdminRequestController, "when administering the holding pen" do assigns[:request_hidden_user_explanation].should include("not a valid FOI") end - it "hides requests and sends a notification email that it has done so" do - ir = info_requests(:fancy_dog_request) - post :hide_request, :id => ir.id, :explanation => "Foo", :reason => "vexatious" - ir.reload - ir.prominence.should == "requester_only" - ir.described_state.should == "vexatious" - deliveries = ActionMailer::Base.deliveries - deliveries.size.should == 1 - mail = deliveries[0] - mail.body.should =~ /Foo/ + describe 'when hiding requests' do + + it "hides requests and sends a notification email that it has done so" do + ir = info_requests(:fancy_dog_request) + post :hide_request, :id => ir.id, :explanation => "Foo", :reason => "vexatious" + ir.reload + ir.prominence.should == "requester_only" + ir.described_state.should == "vexatious" + deliveries = ActionMailer::Base.deliveries + deliveries.size.should == 1 + mail = deliveries[0] + mail.body.should =~ /Foo/ + end + + it 'expires the file cache for the request' do + ir = info_requests(:fancy_dog_request) + @controller.should_receive(:expire_for_request).with(ir) + post :hide_request, :id => ir.id, :explanation => "Foo", :reason => "vexatious" + end + + describe 'when hiding an external request' do + + before do + @controller.stub!(:expire_for_request) + @info_request = mock_model(InfoRequest, :prominence= => nil, + :log_event => nil, + :set_described_state => nil, + :save! => nil, + :user => nil, + :user_name => 'External User', + :is_external? => true) + InfoRequest.stub!(:find).with(@info_request.id.to_s).and_return(@info_request) + @default_params = { :id => @info_request.id, + :explanation => 'Foo', + :reason => 'vexatious' } + end + + def make_request(params=@default_params) + post :hide_request, params + end + + it 'should redirect the the admin page for the request' do + make_request + response.should redirect_to(:controller => 'admin_request', + :action => 'show', + :id => @info_request.id) + end + + it 'should set the request prominence to "requester_only"' do + @info_request.should_receive(:prominence=).with('requester_only') + @info_request.should_receive(:save!) + make_request + end + + it 'should not send a notification email' do + ContactMailer.should_not_receive(:deliver_from_admin_message) + make_request + end + + it 'should add a notice to the flash saying that the request has been hidden' do + make_request + response.flash[:notice].should == "This external request has been hidden" + end + + it 'should expire the file cache for the request' do + @controller.should_receive(:expire_for_request) + make_request + end + end + end end diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 530e9b2c3..6e5498a9f 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -7,7 +7,7 @@ describe RequestController, "when listing recent requests" do load_raw_emails_data rebuild_xapian_index end - + it "should be successful" do get :list, :view => 'all' response.should be_success @@ -21,7 +21,7 @@ describe RequestController, "when listing recent requests" do it "should filter requests" do get :list, :view => 'all' assigns[:list_results].map(&:info_request).should =~ InfoRequest.all - + # default sort order is the request with the most recently created event first assigns[:list_results].map(&:info_request).should == InfoRequest.all( :order => "(select max(info_request_events.created_at) from info_request_events where info_request_events.info_request_id = info_requests.id) DESC") @@ -45,15 +45,15 @@ describe RequestController, "when listing recent requests" do it "should filter requests by date" do # The semantics of the search are that it finds any InfoRequest # that has any InfoRequestEvent created in the specified range - + get :list, :view => 'all', :request_date_before => '13/10/2007' assigns[:list_results].map(&:info_request).should =~ InfoRequest.all( :conditions => "id in (select info_request_id from info_request_events where created_at < '2007-10-13'::date)") - + get :list, :view => 'all', :request_date_after => '13/10/2007' assigns[:list_results].map(&:info_request).should =~ InfoRequest.all( :conditions => "id in (select info_request_id from info_request_events where created_at > '2007-10-13'::date)") - + get :list, :view => 'all', :request_date_after => '13/10/2007', :request_date_before => '01/11/2007' assigns[:list_results].map(&:info_request).should =~ InfoRequest.all( :conditions => "id in (select info_request_id from info_request_events where created_at between '2007-10-13'::date and '2007-11-01'::date)") @@ -74,7 +74,7 @@ describe RequestController, "when listing recent requests" do it "should list internal_review requests as unresolved ones" do get :list, :view => 'awaiting' - + # This doesn’t precisely duplicate the logic of the actual # query, but it is close enough to give the same result with # the current set of test data. @@ -88,22 +88,22 @@ describe RequestController, "when listing recent requests" do where later_events.created_at > info_request_events.created_at and later_events.info_request_id = info_request_events.info_request_id )") - - + + get :list, :view => 'awaiting' assigns[:list_results].map(&:info_request).include?(info_requests(:fancy_dog_request)).should == false - + event = info_request_events(:useless_incoming_message_event) event.described_state = event.calculated_state = "internal_review" event.save! rebuild_xapian_index - + get :list, :view => 'awaiting' assigns[:list_results].map(&:info_request).include?(info_requests(:fancy_dog_request)).should == true end it "should assign the first page of results" do - xap_results = mock_model(ActsAsXapian::Search, + xap_results = mock_model(ActsAsXapian::Search, :results => (1..25).to_a.map { |m| { :model => m } }, :matches_estimated => 1000000) @@ -114,8 +114,9 @@ describe RequestController, "when listing recent requests" do assigns[:list_results].size.should == 25 assigns[:show_no_more_than].should == RequestController::MAX_RESULTS end + it "should return 404 for pages we don't want to serve up" do - xap_results = mock_model(ActsAsXapian::Search, + xap_results = mock_model(ActsAsXapian::Search, :results => (1..25).to_a.map { |m| { :model => m } }, :matches_estimated => 1000000) lambda { @@ -123,6 +124,12 @@ describe RequestController, "when listing recent requests" do }.should raise_error(ActiveRecord::RecordNotFound) end + it 'should not raise an error for a page param of less than zero, but should treat it as + a param of 1' do + lambda{ get :list, :view => 'all', :page => "-1" }.should_not raise_error + assigns[:page].should == 1 + end + end describe RequestController, "when changing things that appear on the request page" do @@ -172,7 +179,7 @@ describe RequestController, "when changing things that appear on the request pag # XXX really, CensorRules should execute expiry logic as part # of the after_save of the model. Currently this is part of # the AdminCensorRuleController logic, so must be tested from - # there. Leaving this stub test in place as a reminder + # there. Leaving this stub test in place as a reminder end it "should purge the downstream cache when something is hidden by an admin" do ir = info_requests(:fancy_dog_request) @@ -194,7 +201,7 @@ end describe RequestController, "when showing one request" do integrate_views - + before(:each) do load_raw_emails_data FileUtils.rm_rf File.join(File.dirname(__FILE__), "../../cache/zips") @@ -226,7 +233,19 @@ describe RequestController, "when showing one request" do response.should redirect_to(:action => 'show', :url_title => info_requests(:naughty_chicken_request).url_title) end - + describe 'when showing an external request' do + + it 'should be successful with no logged in user' do + get :show, { :url_title => 'balalas' }, { :user_id => nil } + response.should be_success + end + + it 'should be successful when logged in as an admin user' do + get :show, { :url_title => 'balalas' }, { :user_id => users(:admin_user).id } + response.should be_success + end + end + describe 'when handling an update_status parameter' do it 'should assign the "update status" flag to the view as true if the parameter is present' do get :show, :url_title => 'why_do_you_have_such_a_fancy_dog', :update_status => 1 @@ -237,26 +256,26 @@ describe RequestController, "when showing one request" do get :show, :url_title => 'why_do_you_have_such_a_fancy_dog' assigns[:update_status].should be_false end - + it 'should require login' do session[:user_id] = nil get :show, :url_title => 'why_do_you_have_such_a_fancy_dog', :update_status => 1 post_redirect = PostRedirect.get_last_post_redirect response.should redirect_to(:controller => 'user', :action => 'signin', :token => post_redirect.token) end - + it 'should work if logged in as the requester' do session[:user_id] = users(:bob_smith_user).id get :show, :url_title => 'why_do_you_have_such_a_fancy_dog', :update_status => 1 response.should render_template "request/show" end - + it 'should not work if logged in as not the requester' do session[:user_id] = users(:silly_name_user).id get :show, :url_title => 'why_do_you_have_such_a_fancy_dog', :update_status => 1 response.should render_template "user/wrong_user" end - + it 'should work if logged in as an admin user' do session[:user_id] = users(:admin_user).id get :show, :url_title => 'why_do_you_have_such_a_fancy_dog', :update_status => 1 @@ -264,8 +283,8 @@ describe RequestController, "when showing one request" do end end - describe 'when handling incoming mail' do - + describe 'when handling incoming mail' do + integrate_views it "should receive incoming messages, send email to creator, and show them" do @@ -284,7 +303,7 @@ describe RequestController, "when showing one request" do get :show, :url_title => 'why_do_you_have_such_a_fancy_dog' (assigns[:info_request_events].size - size_before).should == 1 end - + it "should download attachments" do ir = info_requests(:fancy_dog_request) ir.incoming_messages.each { |x| x.parse_raw_email!(true) } @@ -293,31 +312,43 @@ describe RequestController, "when showing one request" do response.content_type.should == "text/html" size_before = assigns[:info_request_events].size - ir = info_requests(:fancy_dog_request) + ir = info_requests(:fancy_dog_request) receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) get :show, :url_title => 'why_do_you_have_such_a_fancy_dog' (assigns[:info_request_events].size - size_before).should == 1 ir.reload - + get :get_attachment, :incoming_message_id => ir.incoming_messages[1].id, :id => ir.id, :part => 2, :file_name => ['hello.txt'], :skip_cache => 1 response.content_type.should == "text/plain" response.should have_text(/Second hello/) - + get :get_attachment, :incoming_message_id => ir.incoming_messages[1].id, :id => ir.id, :part => 3, :file_name => ['hello.txt'], :skip_cache => 1 response.content_type.should == "text/plain" response.should have_text(/First hello/) end + it 'should cache an attachment on a request with normal prominence' do + ir = info_requests(:fancy_dog_request) + receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) + ir.reload + @controller.should_receive(:foi_fragment_cache_write) + get :get_attachment, :incoming_message_id => ir.incoming_messages[1].id, + :id => ir.id, + :part => 2, + :file_name => ['hello.txt'] + + end + it "should convert message body to UTF8" do - ir = info_requests(:fancy_dog_request) + ir = info_requests(:fancy_dog_request) receive_incoming_mail('iso8859_2_raw_email.email', ir.incoming_email) get :show, :url_title => 'why_do_you_have_such_a_fancy_dog' response.should have_text(/tënde/u) end it "should generate valid HTML verson of plain text attachments" do - ir = info_requests(:fancy_dog_request) + ir = info_requests(:fancy_dog_request) receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) ir.reload get :get_attachment_as_html, :incoming_message_id => ir.incoming_messages[1].id, :id => ir.id, :part => 2, :file_name => ['hello.txt.html'], :skip_cache => 1 @@ -336,7 +367,7 @@ describe RequestController, "when showing one request" do # # https://github.com/sebbacon/alaveteli/issues/351 it "should return 404 for ugly URLs containing a request id that isn't an integer" do - ir = info_requests(:fancy_dog_request) + ir = info_requests(:fancy_dog_request) receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) ir.reload ugly_id = "55195" @@ -362,7 +393,7 @@ describe RequestController, "when showing one request" do receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) ir.reload ugly_id = "%d95" % [info_requests(:naughty_chicken_request).id] - + lambda { get :get_attachment, :incoming_message_id => ir.incoming_messages[1].id, :id => ugly_id, :part => 2, :file_name => ['hello.txt.html'], :skip_cache => 1 }.should raise_error(ActiveRecord::RecordNotFound) @@ -382,7 +413,7 @@ describe RequestController, "when showing one request" do end it "should generate valid HTML verson of PDF attachments" do - ir = info_requests(:fancy_dog_request) + ir = info_requests(:fancy_dog_request) receive_incoming_mail('incoming-request-pdf-attachment.email', ir.incoming_email) ir.reload get :get_attachment_as_html, :incoming_message_id => ir.incoming_messages[1].id, :id => ir.id, :part => 2, :file_name => ['fs_50379341.pdf.html'], :skip_cache => 1 @@ -391,7 +422,7 @@ describe RequestController, "when showing one request" do end it "should not cause a reparsing of the raw email, even when the result would be a 404" do - ir = info_requests(:fancy_dog_request) + ir = info_requests(:fancy_dog_request) receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) ir.reload attachment = IncomingMessage.get_attachment_by_url_part_number(ir.incoming_messages[1].get_attachments_for_display, 2) @@ -405,14 +436,14 @@ describe RequestController, "when showing one request" do lambda { get :get_attachment_as_html, :incoming_message_id => ir.incoming_messages[1].id, :id => ir.id, :part => 2, :file_name => ['hello.txt.baz.html'], :skip_cache => 1 }.should raise_error(ActiveRecord::RecordNotFound) - + attachment = IncomingMessage.get_attachment_by_url_part_number(ir.incoming_messages[1].get_attachments_for_display, 2) attachment.body.should have_text(/Second hello/) # ...nor should asking for it by its correct filename... get :get_attachment_as_html, :incoming_message_id => ir.incoming_messages[1].id, :id => ir.id, :part => 2, :file_name => ['hello.txt.html'], :skip_cache => 1 response.should_not have_text(/Third hello/) - + # ...but if we explicitly ask for attachments to be extracted, then they should be force = true ir.incoming_messages[1].parse_raw_email!(force) @@ -426,14 +457,14 @@ describe RequestController, "when showing one request" do ir = info_requests(:fancy_dog_request) receive_incoming_mail('incoming-request-attachment-unknown-extension.email', ir.incoming_email) ir.reload - + get :get_attachment, :incoming_message_id => ir.incoming_messages[1].id, :id => ir.id, :part => 2, :file_name => ['hello.qwglhm'], :skip_cache => 1 response.content_type.should == "application/octet-stream" response.should have_text(/an unusual sort of file/) end it "should not download attachments with wrong file name" do - ir = info_requests(:fancy_dog_request) + ir = info_requests(:fancy_dog_request) receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) lambda { @@ -443,7 +474,7 @@ describe RequestController, "when showing one request" do end it "should censor attachments downloaded as binary" do - ir = info_requests(:fancy_dog_request) + ir = info_requests(:fancy_dog_request) censor_rule = CensorRule.new() censor_rule.text = "Second" @@ -451,7 +482,7 @@ describe RequestController, "when showing one request" do censor_rule.last_edit_editor = "unknown" censor_rule.last_edit_comment = "none" ir.censor_rules << censor_rule - + begin receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) @@ -464,7 +495,7 @@ describe RequestController, "when showing one request" do end it "should censor with rules on the user (rather than the request)" do - ir = info_requests(:fancy_dog_request) + ir = info_requests(:fancy_dog_request) censor_rule = CensorRule.new() censor_rule.text = "Second" @@ -486,7 +517,7 @@ describe RequestController, "when showing one request" do end it "should censor attachment names" do - ir = info_requests(:fancy_dog_request) + ir = info_requests(:fancy_dog_request) receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) # XXX this is horrid, but don't know a better way. If we @@ -505,7 +536,7 @@ describe RequestController, "when showing one request" do # so at this point, assigns[:info_request].incoming_messages[1].get_attachments_for_display is returning stuff, but the equivalent thing in the template isn't. # but something odd is that the above is return a whole load of attachments which aren't there in the controller - response.body.should have_tag("p.attachment strong", /hello.txt/m) + response.body.should have_tag("p.attachment strong", /hello.txt/m) censor_rule = CensorRule.new() censor_rule.text = "hello.txt" @@ -521,38 +552,48 @@ describe RequestController, "when showing one request" do end end - it "should make a zipfile available, which has a different URL when it changes" do - title = 'why_do_you_have_such_a_fancy_dog' - ir = info_requests(:fancy_dog_request) - session[:user_id] = ir.user.id # bob_smith_user - get :download_entire_request, :url_title => title - assigns[:url_path].should have_text(/#{title}.zip$/) - old_path = assigns[:url_path] - response.location.should have_text(/#{assigns[:url_path]}$/) - zipfile = Zip::ZipFile.open(File.join(File.dirname(__FILE__), "../../cache/zips", old_path)) { |zipfile| - zipfile.count.should == 1 # just the message - } - receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) - get :download_entire_request, :url_title => title - assigns[:url_path].should have_text(/#{title}.zip$/) - old_path = assigns[:url_path] - response.location.should have_text(/#{assigns[:url_path]}$/) - zipfile = Zip::ZipFile.open(File.join(File.dirname(__FILE__), "../../cache/zips", old_path)) { |zipfile| - zipfile.count.should == 3 # the message plus two "hello.txt" files - } - - # The path of the zip file is based on the hash of the timestamp of the last request - # in the thread, so we wait for a second to make sure this one will have a different - # timestamp than the previous. - sleep 1 - receive_incoming_mail('incoming-request-attachment-unknown-extension.email', ir.incoming_email) - get :download_entire_request, :url_title => title - assigns[:url_path].should have_text(/#{title}.zip$/) - assigns[:url_path].should_not == old_path - response.location.should have_text(/#{assigns[:url_path]}/) - zipfile = Zip::ZipFile.open(File.join(File.dirname(__FILE__), "../../cache/zips", assigns[:url_path])) { |zipfile| - zipfile.count.should == 5 # the message, two hello.txt, the unknown attachment, and its empty message - } + describe 'when making a zipfile available' do + + it "should have a different zipfile URL when the request changes" do + title = 'why_do_you_have_such_a_fancy_dog' + ir = info_requests(:fancy_dog_request) + session[:user_id] = ir.user.id # bob_smith_user + get :download_entire_request, :url_title => title + assigns[:url_path].should have_text(/#{title}.zip$/) + old_path = assigns[:url_path] + response.location.should have_text(/#{assigns[:url_path]}$/) + zipfile = Zip::ZipFile.open(File.join(File.dirname(__FILE__), "../../cache/zips", old_path)) { |zipfile| + zipfile.count.should == 1 # just the message + } + receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) + get :download_entire_request, :url_title => title + assigns[:url_path].should have_text(/#{title}.zip$/) + old_path = assigns[:url_path] + response.location.should have_text(/#{assigns[:url_path]}$/) + zipfile = Zip::ZipFile.open(File.join(File.dirname(__FILE__), "../../cache/zips", old_path)) { |zipfile| + zipfile.count.should == 3 # the message plus two "hello.txt" files + } + + # The path of the zip file is based on the hash of the timestamp of the last request + # in the thread, so we wait for a second to make sure this one will have a different + # timestamp than the previous. + sleep 1 + receive_incoming_mail('incoming-request-attachment-unknown-extension.email', ir.incoming_email) + get :download_entire_request, :url_title => title + assigns[:url_path].should have_text(/#{title}.zip$/) + assigns[:url_path].should_not == old_path + response.location.should have_text(/#{assigns[:url_path]}/) + zipfile = Zip::ZipFile.open(File.join(File.dirname(__FILE__), "../../cache/zips", assigns[:url_path])) { |zipfile| + zipfile.count.should == 5 # the message, two hello.txt, the unknown attachment, and its empty message + } + end + + it 'should successfully make a zipfile for an external request' do + info_request = info_requests(:external_request) + get :download_entire_request, { :url_title => info_request.url_title }, + { :user_id => users(:bob_smith_user) } + response.location.should have_text(/#{assigns[:url_path]}/) + end end end end @@ -617,11 +658,20 @@ describe RequestController, "when changing prominence of a request" do session[:user_id] = users(:admin_user).id get :show, :url_title => 'why_do_you_have_such_a_fancy_dog' response.should render_template('show') + end + it 'should not cache an attachment on a request whose prominence is requester_only when showing + the request to the requester or admin' do + ir = info_requests(:fancy_dog_request) + ir.prominence = 'requester_only' + ir.save! + session[:user_id] = ir.user.id # bob_smith_user + @controller.should_not_receive(:foi_fragment_cache_write) + get :show, :url_title => 'why_do_you_have_such_a_fancy_dog' end it "should not download attachments if hidden" do - ir = info_requests(:fancy_dog_request) + ir = info_requests(:fancy_dog_request) ir.prominence = 'hidden' ir.save! receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) @@ -637,7 +687,7 @@ describe RequestController, "when changing prominence of a request" do end end - + # XXX do this for invalid ids # it "should render 404 file" do # response.should render_template("#{Rails.root}/public/404.html") @@ -651,11 +701,11 @@ describe RequestController, "when searching for an authority" do before do @user = users(:bob_smith_user) end - + it "should return nothing for the empty query string" do session[:user_id] = @user.id get :select_authority, :query => "" - + response.should render_template('select_authority') assigns[:xapian_requests].should == nil end @@ -663,7 +713,7 @@ describe RequestController, "when searching for an authority" do it "should return matching bodies" do session[:user_id] = @user.id get :select_authority, :query => "Quango" - + response.should render_template('select_authority') assigns[:xapian_requests].results.size == 1 assigns[:xapian_requests].results[0][:model].name.should == public_bodies(:geraldine_public_body).name @@ -691,7 +741,7 @@ describe RequestController, "when creating a new request" do @user = users(:bob_smith_user) @body = public_bodies(:geraldine_public_body) end - + it "should redirect to front page if no public body specified" do get :new response.should redirect_to(:controller => 'general', :action => 'frontpage') @@ -725,7 +775,7 @@ describe RequestController, "when creating a new request" do end it "should redirect to sign in page when input is good and nobody is logged in" do - params = { :info_request => { :public_body_id => @body.id, + params = { :info_request => { :public_body_id => @body.id, :title => "Why is your quango called Geraldine?", :tag_string => "" }, :outgoing_message => { :body => "This is a silly letter. It is too short to be interesting." }, :submitted_new_request => 1, :preview => 0 @@ -738,7 +788,7 @@ describe RequestController, "when creating a new request" do it "should show preview when input is good" do session[:user_id] = @user.id - post :new, { :info_request => { :public_body_id => @body.id, + post :new, { :info_request => { :public_body_id => @body.id, :title => "Why is your quango called Geraldine?", :tag_string => "" }, :outgoing_message => { :body => "This is a silly letter. It is too short to be interesting." }, :submitted_new_request => 1, :preview => 1 @@ -757,7 +807,7 @@ describe RequestController, "when creating a new request" do it "should create the request and outgoing message, and send the outgoing message by email, and redirect to request page when input is good and somebody is logged in" do session[:user_id] = @user.id - post :new, :info_request => { :public_body_id => @body.id, + post :new, :info_request => { :public_body_id => @body.id, :title => "Why is your quango called Geraldine?", :tag_string => "" }, :outgoing_message => { :body => "This is a silly letter. It is too short to be interesting." }, :submitted_new_request => 1, :preview => 0 @@ -784,7 +834,7 @@ describe RequestController, "when creating a new request" do session[:user_id] = @user.id # We use raw_body here, so white space is the same - post :new, :info_request => { :public_body_id => info_requests(:fancy_dog_request).public_body_id, + post :new, :info_request => { :public_body_id => info_requests(:fancy_dog_request).public_body_id, :title => info_requests(:fancy_dog_request).title }, :outgoing_message => { :body => info_requests(:fancy_dog_request).outgoing_messages[0].raw_body}, :submitted_new_request => 1, :preview => 0, :mouse_house => 1 @@ -794,12 +844,12 @@ describe RequestController, "when creating a new request" do it "should let you submit another request with the same title" do session[:user_id] = @user.id - post :new, :info_request => { :public_body_id => @body.id, + post :new, :info_request => { :public_body_id => @body.id, :title => "Why is your quango called Geraldine?", :tag_string => "" }, :outgoing_message => { :body => "This is a silly letter. It is too short to be interesting." }, :submitted_new_request => 1, :preview => 0 - post :new, :info_request => { :public_body_id => @body.id, + post :new, :info_request => { :public_body_id => @body.id, :title => "Why is your quango called Geraldine?", :tag_string => "" }, :outgoing_message => { :body => "This is a sensible letter. It is too long to be boring." }, :submitted_new_request => 1, :preview => 0 @@ -814,32 +864,32 @@ describe RequestController, "when creating a new request" do response.should redirect_to(:action => 'show', :url_title => ir2.url_title) end - + it 'should respect the rate limit' do # Try to create three requests in succession. # (The limit set in config/test.yml is two.) session[:user_id] = users(:robin_user) - post :new, :info_request => { :public_body_id => @body.id, + post :new, :info_request => { :public_body_id => @body.id, :title => "What is the answer to the ultimate question?", :tag_string => "" }, :outgoing_message => { :body => "Please supply the answer from your files." }, :submitted_new_request => 1, :preview => 0 response.should redirect_to(:action => 'show', :url_title => 'what_is_the_answer_to_the_ultima') - - post :new, :info_request => { :public_body_id => @body.id, + + post :new, :info_request => { :public_body_id => @body.id, :title => "Why did the chicken cross the road?", :tag_string => "" }, :outgoing_message => { :body => "Please send me all the relevant documents you hold." }, :submitted_new_request => 1, :preview => 0 response.should redirect_to(:action => 'show', :url_title => 'why_did_the_chicken_cross_the_ro') - post :new, :info_request => { :public_body_id => @body.id, + post :new, :info_request => { :public_body_id => @body.id, :title => "What's black and white and red all over?", :tag_string => "" }, :outgoing_message => { :body => "Please send all minutes of meetings and email records that address this question." }, :submitted_new_request => 1, :preview => 0 response.should render_template('user/rate_limited') end - + it 'should ignore the rate limit for specified users' do # Try to create three requests in succession. # (The limit set in config/test.yml is two.) @@ -847,20 +897,20 @@ describe RequestController, "when creating a new request" do users(:robin_user).no_limit = true users(:robin_user).save! - post :new, :info_request => { :public_body_id => @body.id, + post :new, :info_request => { :public_body_id => @body.id, :title => "What is the answer to the ultimate question?", :tag_string => "" }, :outgoing_message => { :body => "Please supply the answer from your files." }, :submitted_new_request => 1, :preview => 0 response.should redirect_to(:action => 'show', :url_title => 'what_is_the_answer_to_the_ultima') - - post :new, :info_request => { :public_body_id => @body.id, + + post :new, :info_request => { :public_body_id => @body.id, :title => "Why did the chicken cross the road?", :tag_string => "" }, :outgoing_message => { :body => "Please send me all the relevant documents you hold." }, :submitted_new_request => 1, :preview => 0 response.should redirect_to(:action => 'show', :url_title => 'why_did_the_chicken_cross_the_ro') - post :new, :info_request => { :public_body_id => @body.id, + post :new, :info_request => { :public_body_id => @body.id, :title => "What's black and white and red all over?", :tag_string => "" }, :outgoing_message => { :body => "Please send all minutes of meetings and email records that address this question." }, :submitted_new_request => 1, :preview => 0 @@ -869,7 +919,7 @@ describe RequestController, "when creating a new request" do end -# These go with the previous set, but use mocks instead of fixtures. +# These go with the previous set, but use mocks instead of fixtures. # TODO harmonise these describe RequestController, "when making a new request" do @@ -956,7 +1006,7 @@ describe RequestController, "when viewing an individual response for reply/follo end it "should not show individual responses if request hidden, even if request owner" do - ir = info_requests(:fancy_dog_request) + ir = info_requests(:fancy_dog_request) ir.prominence = 'hidden' ir.save! @@ -968,7 +1018,7 @@ end describe RequestController, "when classifying an information request" do - before(:each) do + before(:each) do @dog_request = info_requests(:fancy_dog_request) @dog_request.stub!(:is_old_unclassified?).and_return(false) InfoRequest.stub!(:find).and_return(@dog_request) @@ -976,9 +1026,9 @@ describe RequestController, "when classifying an information request" do end def post_status(status) - post :describe_state, :incoming_message => { :described_state => status }, - :id => @dog_request.id, - :last_info_request_event_id => @dog_request.last_event_id_needing_description, + post :describe_state, :incoming_message => { :described_state => status }, + :id => @dog_request.id, + :last_info_request_event_id => @dog_request.last_event_id_needing_description, :submitted_describe_state => 1 end @@ -988,79 +1038,79 @@ describe RequestController, "when classifying an information request" do response.should redirect_to(:controller => 'user', :action => 'signin', :token => post_redirect.token) end - it 'should ask whether the request is old and unclassified' do + it 'should ask whether the request is old and unclassified' do @dog_request.should_receive(:is_old_unclassified?) post_status('rejected') end - + it "should not classify the request if logged in as the wrong user" do session[:user_id] = users(:silly_name_user).id post_status('rejected') response.should render_template('user/wrong_user') end - - describe 'when the request is old and unclassified' do - - before do + + describe 'when the request is old and unclassified' do + + before do @dog_request.stub!(:is_old_unclassified?).and_return(true) RequestMailer.stub!(:deliver_old_unclassified_updated) end - - describe 'when the user is not logged in' do - - it 'should require login' do + + describe 'when the user is not logged in' do + + it 'should require login' do session[:user_id] = nil post_status('rejected') post_redirect = PostRedirect.get_last_post_redirect response.should redirect_to(:controller => 'user', :action => 'signin', :token => post_redirect.token) end - + end - - describe 'when the user is logged in as a different user' do - + + describe 'when the user is logged in as a different user' do + before do @other_user = mock_model(User) session[:user_id] = users(:silly_name_user).id end - + it 'should classify the request' do - @dog_request.stub!(:calculate_status).and_return('rejected') + @dog_request.stub!(:calculate_status).and_return('rejected') @dog_request.should_receive(:set_described_state).with('rejected') post_status('rejected') end - - it 'should log a status update event' do - expected_params = {:user_id => users(:silly_name_user).id, - :old_described_state => 'waiting_response', + + it 'should log a status update event' 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) post_status('rejected') end - - it 'should send an email to the requester letting them know someone has updated the status of their request' do + + it 'should send an email to the requester letting them know someone has updated the status of their request' do RequestMailer.should_receive(:deliver_old_unclassified_updated) post_status('rejected') end - - it 'should redirect to the request page' do + + it 'should redirect to the request page' do post_status('rejected') response.should redirect_to(:action => 'show', :controller => 'request', :url_title => @dog_request.url_title) end - - it 'should show a message thanking the user for a good deed' do + + it 'should show a message thanking the user for a good deed' do post_status('rejected') flash[:notice].should == 'Thank you for updating this request!' end - + end end - - describe 'when logged in as an admin user who is not the actual requester' do - - before do + + describe 'when logged in as an admin user who is not the actual requester' do + + before do @admin_user = users(:admin_user) session[:user_id] = @admin_user.id @dog_request = info_requests(:fancy_dog_request) @@ -1068,39 +1118,39 @@ describe RequestController, "when classifying an information request" do @dog_request.stub!(:each).and_return([@dog_request]) end - it 'should update the status of the request' do + it 'should update the status of the request' do @dog_request.stub!(:calculate_status).and_return('rejected') @dog_request.should_receive(:set_described_state).with('rejected') post_status('rejected') end - - it 'should log a status update event' do - expected_params = {:user_id => @admin_user.id, - :old_described_state => 'waiting_response', + + it 'should log a status update event' do + 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) post_status('rejected') end - it 'should send an email to the requester letting them know someone has updated the status of their request' do + it 'should send an email to the requester letting them know someone has updated the status of their request' do RequestMailer.should_receive(:deliver_old_unclassified_updated) post_status('rejected') end - it 'should redirect to the request page' do + it 'should redirect to the request page' do post_status('rejected') response.should redirect_to(:action => 'show', :controller => 'request', :url_title => @dog_request.url_title) end - it 'should show a message thanking the user for a good deed' do + it 'should show a message thanking the user for a good deed' do post_status('rejected') flash[:notice].should == 'Thank you for updating this request!' end end - describe 'when logged in as an admin user who is also the actual requester' do - - before do + describe 'when logged in as an admin user who is also the actual requester' do + + before do @admin_user = users(:admin_user) session[:user_id] = @admin_user.id @dog_request = info_requests(:fancy_dog_request) @@ -1110,43 +1160,43 @@ describe RequestController, "when classifying an information request" do @dog_request.stub!(:each).and_return([@dog_request]) end - it 'should update the status of the request' do + it 'should update the status of the request' do @dog_request.stub!(:calculate_status).and_return('rejected') @dog_request.should_receive(:set_described_state).with('rejected') post_status('rejected') end - - it 'should not log a status update event' do + + it 'should not log a status update event' do @dog_request.should_not_receive(:log_event) post_status('rejected') end - it 'should not send an email to the requester letting them know someone has updated the status of their request' do + it 'should not send an email to the requester letting them know someone has updated the status of their request' do RequestMailer.should_not_receive(:deliver_old_unclassified_updated) post_status('rejected') end - - it 'should say it is showing advice as to what to do next' do + + it 'should say it is showing advice as to what to do next' do post_status('rejected') - flash[:notice].should match(/Here is what to do now/) + flash[:notice].should match(/Here is what to do now/) end - - it 'should redirect to the unhappy page' do + + it 'should redirect to the unhappy page' do post_status('rejected') response.should redirect_to(:controller => 'help', :action => 'unhappy', :url_title => @dog_request.url_title) end end - - describe 'when logged in as the requestor' do - - before do + + describe 'when logged in as the requestor' do + + before do @request_owner = users(:bob_smith_user) session[:user_id] = @request_owner.id @dog_request.awaiting_description.should == true @dog_request.stub!(:each).and_return([@dog_request]) end - + it "should successfully classify response if logged in as user controlling request" do post_status('rejected') response.should redirect_to(:controller => 'help', :action => 'unhappy', :url_title => @dog_request.url_title) @@ -1157,16 +1207,16 @@ describe RequestController, "when classifying an information request" do @dog_request.get_last_response_event.calculated_state.should == 'rejected' end - it 'should not log a status update event' do + it 'should not log a status update event' do @dog_request.should_not_receive(:log_event) post_status('rejected') end - - it 'should not send an email to the requester letting them know someone has updated the status of their request' do + + it 'should not send an email to the requester letting them know someone has updated the status of their request' do RequestMailer.should_not_receive(:deliver_old_unclassified_updated) post_status('rejected') end - + it "should send email when classified as requires_admin" do post :describe_state, :incoming_message => { :described_state => "requires_admin" }, :id => @dog_request.id, :incoming_message_id => incoming_messages(:useless_incoming_message), :last_info_request_event_id => @dog_request.last_event_id_needing_description, :submitted_describe_state => 1 response.should redirect_to(:controller => 'help', :action => 'contact') @@ -1183,12 +1233,12 @@ describe RequestController, "when classifying an information request" do mail.from_addrs.first.to_s.should == @request_owner.name_and_email end - it 'should say it is showing advice as to what to do next' do + it 'should say it is showing advice as to what to do next' do post_status('rejected') - flash[:notice].should match(/Here is what to do now/) + flash[:notice].should match(/Here is what to do now/) end - - it 'should redirect to the unhappy page' do + + it 'should redirect to the unhappy page' do post_status('rejected') response.should redirect_to(:controller => 'help', :action => 'unhappy', :url_title => @dog_request.url_title) end @@ -1200,15 +1250,15 @@ describe RequestController, "when classifying an information request" do RequestController.send(:require, File.expand_path(File.join(File.dirname(__FILE__), '..', 'models', 'customstates'))) RequestController.send(:include, RequestControllerCustomStates) RequestController.class_eval('@@custom_states_loaded = true') - Time.stub!(:now).and_return(Time.utc(2007, 11, 10, 00, 01)) + Time.stub!(:now).and_return(Time.utc(2007, 11, 10, 00, 01)) post_status('deadline_extended') flash[:notice].should == 'Authority has requested extension of the deadline.' end end - - describe 'when redirecting after a successful status update by the request owner' do - - before do + + describe 'when redirecting after a successful status update by the request owner' do + + before do @request_owner = users(:bob_smith_user) session[:user_id] = @request_owner.id @dog_request = info_requests(:fancy_dog_request) @@ -1228,12 +1278,12 @@ describe RequestController, "when classifying an information request" do def unhappy_url "help/unhappy/#{@dog_request.url_title}" end - + def expect_redirect(status, redirect_path) post_status(status) response.should redirect_to("http://test.host/#{redirect_path}") end - + it 'should redirect to the "request url" with a message in the right tense when status is updated to "waiting response" and the response is not overdue' do @dog_request.stub!(:date_response_required_by).and_return(Time.now.to_date+1) @dog_request.stub!(:date_very_overdue_after).and_return(Time.now.to_date+40) @@ -1241,15 +1291,15 @@ describe RequestController, "when classifying an information request" do expect_redirect("waiting_response", "request/#{@dog_request.url_title}") flash[:notice].should match(/should get a response/) end - - it 'should redirect to the "request url" with a message in the right tense when status is updated to "waiting response" and the response is overdue' do + + it 'should redirect to the "request url" with a message in the right tense when status is updated to "waiting response" and the response is overdue' do @dog_request.stub!(:date_response_required_by).and_return(Time.now.to_date-1) @dog_request.stub!(:date_very_overdue_after).and_return(Time.now.to_date+40) expect_redirect('waiting_response', request_url) flash[:notice].should match(/should have got a response/) end - it 'should redirect to the "request url" with a message in the right tense when status is updated to "waiting response" and the response is overdue' do + it 'should redirect to the "request url" with a message in the right tense when status is updated to "waiting response" and the response is overdue' do @dog_request.stub!(:date_response_required_by).and_return(Time.now.to_date-2) @dog_request.stub!(:date_very_overdue_after).and_return(Time.now.to_date-1) expect_redirect('waiting_response', unhappy_url) @@ -1257,54 +1307,54 @@ describe RequestController, "when classifying an information request" do flash[:notice].should match(/by more than 40 working days/) flash[:notice].should match(/within 20 working days/) end - - it 'should redirect to the "request url" when status is updated to "not held"' do + + it 'should redirect to the "request url" when status is updated to "not held"' do expect_redirect('not_held', request_url) end - - it 'should redirect to the "request url" when status is updated to "successful"' do + + it 'should redirect to the "request url" when status is updated to "successful"' do expect_redirect('successful', request_url) end - - it 'should redirect to the "unhappy url" when status is updated to "rejected"' do + + it 'should redirect to the "unhappy url" when status is updated to "rejected"' do expect_redirect('rejected', "help/unhappy/#{@dog_request.url_title}") end - - it 'should redirect to the "unhappy url" when status is updated to "partially successful"' do + + it 'should redirect to the "unhappy url" when status is updated to "partially successful"' do expect_redirect('partially_successful', "help/unhappy/#{@dog_request.url_title}") end - - it 'should redirect to the "response url" when status is updated to "waiting clarification" and there is a last response' do + + it 'should redirect to the "response url" when status is updated to "waiting clarification" and there is a last response' do incoming_message = mock_model(IncomingMessage) @dog_request.stub!(:get_last_response).and_return(incoming_message) expect_redirect('waiting_clarification', "request/#{@dog_request.id}/response/#{incoming_message.id}") end - - it 'should redirect to the "response no followup url" when status is updated to "waiting clarification" and there are no events needing description' do + + it 'should redirect to the "response no followup url" when status is updated to "waiting clarification" and there are no events needing description' do @dog_request.stub!(:get_last_response).and_return(nil) expect_redirect('waiting_clarification', "request/#{@dog_request.id}/response") end - it 'should redirect to the "respond to last url" when status is updated to "gone postal"' do + it 'should redirect to the "respond to last url" when status is updated to "gone postal"' do expect_redirect('gone_postal', "request/#{@dog_request.id}/response/#{@dog_request.get_last_response.id}?gone_postal=1") end - - it 'should redirect to the "request url" when status is updated to "internal review"' do + + it 'should redirect to the "request url" when status is updated to "internal review"' do expect_redirect('internal_review', request_url) end - - it 'should redirect to the "help general url" when status is updated to "requires admin"' do + + it 'should redirect to the "help general url" when status is updated to "requires admin"' do expect_redirect('requires_admin', "help/contact") end - - it 'should redirect to the "help general url" when status is updated to "error message"' do + + it 'should redirect to the "help general url" when status is updated to "error message"' do expect_redirect('error_message', "help/contact") end - - it 'should redirect to the "respond to last url url" when status is updated to "user_withdrawn"' do + + it 'should redirect to the "respond to last url url" when status is updated to "user_withdrawn"' do expect_redirect('user_withdrawn', "request/#{@dog_request.id}/response/#{@dog_request.get_last_response.id}") end - + end end @@ -1346,7 +1396,7 @@ describe RequestController, "when sending a followup message" do post :show_response, :outgoing_message => { :body => "What a useless response! You suck.", :what_doing => 'normal_sort'}, :id => info_requests(:fancy_dog_request).id, :incoming_message_id => incoming_messages(:useless_incoming_message), :submitted_followup => 1, :preview => 0, :reedit => "Re-edit this request" response.should render_template('show_response') end - + it "should send the follow up message if you are the right user" do # fake that this is a clarification info_requests(:fancy_dog_request).set_described_state('waiting_clarification') @@ -1378,7 +1428,7 @@ describe RequestController, "when sending a followup message" do # make the followup once post :show_response, :outgoing_message => { :body => "Stop repeating yourself!", :what_doing => 'normal_sort' }, :id => info_requests(:fancy_dog_request).id, :incoming_message_id => incoming_messages(:useless_incoming_message), :submitted_followup => 1 response.should redirect_to(:action => 'show', :url_title => info_requests(:fancy_dog_request).url_title) - + # second time should give an error post :show_response, :outgoing_message => { :body => "Stop repeating yourself!", :what_doing => 'normal_sort' }, :id => info_requests(:fancy_dog_request).id, :incoming_message_id => incoming_messages(:useless_incoming_message), :submitted_followup => 1 # XXX how do I check the error message here? @@ -1392,7 +1442,7 @@ end describe RequestController, "sending overdue request alerts" do integrate_views - + before(:each) do load_raw_emails_data end @@ -1407,7 +1457,7 @@ describe RequestController, "sending overdue request alerts" do chicken_mails = ActionMailer::Base.deliveries.select{|x| x.body =~ /chickens/} chicken_mails.size.should == 1 mail = chicken_mails[0] - + mail.body.should =~ /promptly, as normally/ mail.to_addrs.first.to_s.should == info_requests(:naughty_chicken_request).user.name_and_email @@ -1436,7 +1486,7 @@ describe RequestController, "sending overdue request alerts" do chicken_mails = ActionMailer::Base.deliveries.select{|x| x.body =~ /chickens/} chicken_mails.size.should == 1 mail = chicken_mails[0] - + mail.body.should =~ /promptly, as normally/ mail.to_addrs.first.to_s.should == info_requests(:naughty_chicken_request).user.name_and_email end @@ -1462,7 +1512,7 @@ describe RequestController, "sending overdue request alerts" do chicken_mails = ActionMailer::Base.deliveries.select{|x| x.body =~ /chickens/} chicken_mails.size.should == 1 mail = chicken_mails[0] - + mail.body.should =~ /required by law/ mail.to_addrs.first.to_s.should == info_requests(:naughty_chicken_request).user.name_and_email @@ -1568,9 +1618,9 @@ describe RequestController, "comment alerts" do before(:each) do load_raw_emails_data end - + it "should send an alert (once and once only)" do - # delete ficture comment and make new one, so is in last month (as + # delete fixture comment and make new one, so is in last month (as # alerts are only for comments in last month, see # RequestMailer.alert_comment_on_request) existing_comment = info_requests(:fancy_dog_request).comments[0] @@ -1586,11 +1636,8 @@ describe RequestController, "comment alerts" do mail.to_addrs.first.to_s.should == info_requests(:fancy_dog_request).user.name_and_email mail.body =~ /(http:\/\/.*)/ mail_url = $1 + mail_url.should match("/request/why_do_you_have_such_a_fancy_dog#comment-#{new_comment.id}") - # XXX check mail_url here somehow, can't call comment_url like this: - # mail_url.should == comment_url(comments(:silly_comment)) - - # check if we send again, no more go out deliveries.clear RequestMailer.alert_comment_on_request @@ -1599,7 +1646,7 @@ describe RequestController, "comment alerts" do end it "should not send an alert when you comment on your own request" do - # delete ficture comment and make new one, so is in last month (as + # delete fixture comment and make new one, so is in last month (as # alerts are only for comments in last month, see # RequestMailer.alert_comment_on_request) existing_comment = info_requests(:fancy_dog_request).comments[0] @@ -1614,6 +1661,16 @@ describe RequestController, "comment alerts" do deliveries.size.should == 0 end + it 'should not send an alert for a comment on an external request' do + external_request = info_requests(:external_request) + external_request.add_comment("This external request is interesting", users(:silly_name_user)) + # try to send comment alert + RequestMailer.alert_comment_on_request + + deliveries = ActionMailer::Base.deliveries + deliveries.size.should == 0 + end + it "should send an alert when there are two new comments" do # add two comments - the second one sould be ignored, as is by the user who made the request. # the new comment here, will cause the one in the fixture to be picked up as a new comment by alert_comment_on_request also. @@ -1629,9 +1686,7 @@ describe RequestController, "comment alerts" do mail.to_addrs.first.to_s.should == info_requests(:fancy_dog_request).user.name_and_email mail.body =~ /(http:\/\/.*)/ mail_url = $1 - - # XXX check mail_url here somehow, can't call comment_url like this: - # mail_url.should == comment_url(comments(:silly_comment)) + mail_url.should match("/request/why_do_you_have_such_a_fancy_dog#comment-#{comments(:silly_comment).id}") end @@ -1646,15 +1701,15 @@ describe RequestController, "when viewing comments" do it "should link to the user who submitted it" do session[:user_id] = users(:bob_smith_user).id get :show, :url_title => 'why_do_you_have_such_a_fancy_dog' - response.body.should have_tag("div#comment-1 h2", /Silly.*left an annotation/m) - response.body.should_not have_tag("div#comment-1 h2", /You.*left an annotation/m) + response.body.should have_tag("div#comment-1 h2", /Silly.*left an annotation/m) + response.body.should_not have_tag("div#comment-1 h2", /You.*left an annotation/m) end it "should link to the user who submitted to it, even if it is you" do session[:user_id] = users(:silly_name_user).id get :show, :url_title => 'why_do_you_have_such_a_fancy_dog' - response.body.should have_tag("div#comment-1 h2", /Silly.*left an annotation/m) - response.body.should_not have_tag("div#comment-1 h2", /You.*left an annotation/m) + response.body.should have_tag("div#comment-1 h2", /Silly.*left an annotation/m) + response.body.should_not have_tag("div#comment-1 h2", /You.*left an annotation/m) end end @@ -1666,17 +1721,17 @@ describe RequestController, "authority uploads a response from the web interface before(:each) do # domain after the @ is used for authentication of FOI officers, so to test it # we need a user which isn't at localhost. - @normal_user = User.new(:name => "Mr. Normal", :email => "normal-user@flourish.org", + @normal_user = User.new(:name => "Mr. Normal", :email => "normal-user@flourish.org", :password => PostRedirect.generate_random_token) @normal_user.save! - @foi_officer_user = User.new(:name => "The Geraldine Quango", :email => "geraldine-requests@localhost", + @foi_officer_user = User.new(:name => "The Geraldine Quango", :email => "geraldine-requests@localhost", :password => PostRedirect.generate_random_token) @foi_officer_user.save! end - + it "should require login to view the form to upload" do - @ir = info_requests(:fancy_dog_request) + @ir = info_requests(:fancy_dog_request) @ir.public_body.is_foi_officer?(@normal_user).should == false session[:user_id] = @normal_user.id @@ -1685,7 +1740,7 @@ describe RequestController, "authority uploads a response from the web interface end it "should let you view upload form if you are an FOI officer" do - @ir = info_requests(:fancy_dog_request) + @ir = info_requests(:fancy_dog_request) @ir.public_body.is_foi_officer?(@foi_officer_user).should == true session[:user_id] = @foi_officer_user.id @@ -1694,7 +1749,7 @@ describe RequestController, "authority uploads a response from the web interface end it "should prevent uploads if you are not a requester" do - @ir = info_requests(:fancy_dog_request) + @ir = info_requests(:fancy_dog_request) incoming_before = @ir.incoming_messages.size session[:user_id] = @normal_user.id @@ -1715,10 +1770,14 @@ describe RequestController, "authority uploads a response from the web interface flash[:error].should match(/Please type a message/) end + it 'should 404 for non existent requests' do + lambda{ post :upload_response, :url_title => 'i_dont_exist'}.should raise_error(ActiveRecord::RecordNotFound) + end + # How do I test a file upload in rails? # http://stackoverflow.com/questions/1178587/how-do-i-test-a-file-upload-in-rails it "should let the authority upload a file" do - @ir = info_requests(:fancy_dog_request) + @ir = info_requests(:fancy_dog_request) incoming_before = @ir.incoming_messages.size session[:user_id] = @foi_officer_user.id @@ -1774,7 +1833,7 @@ describe RequestController, "when doing type ahead searches" do response.should render_template('request/_search_ahead.rhtml') assigns[:xapian_requests].should be_nil end - + it "should return a request matching the given keyword, but not users with a matching description" do get :search_typeahead, :q => "chicken" response.should render_template('request/_search_ahead.rhtml') @@ -1793,13 +1852,13 @@ describe RequestController, "when doing type ahead searches" do end it "should not return matches for short words" do - get :search_typeahead, :q => "a" + get :search_typeahead, :q => "a" response.should render_template('request/_search_ahead.rhtml') assigns[:xapian_requests].should be_nil end it "should do partial matches for longer words" do - get :search_typeahead, :q => "chick" + get :search_typeahead, :q => "chick" response.should render_template('request/_search_ahead.rhtml') assigns[:xapian_requests].results.size.should ==1 end @@ -1836,7 +1895,7 @@ describe RequestController, "when showing similar requests" do it "should show similar requests" do badger_request = info_requests(:badger_request) get :similar, :url_title => badger_request.url_title - + # Xapian seems to think *all* the requests are similar assigns[:xapian_object].results.map{|x|x[:model].info_request}.should =~ InfoRequest.all.reject {|x| x == badger_request} end @@ -1860,21 +1919,27 @@ end describe RequestController, "when reporting a request (logged in)" do integrate_views - + before do @user = users(:robin_user) session[:user_id] = @user.id end + it "should 404 for non-existent requests" do + lambda { + post :report_request, :url_title => "hjksfdhjk_louytu_qqxxx" + }.should raise_error(ActiveRecord::RecordNotFound) + end + it "should mark a request as having been reported" do ir = info_requests(:badger_request) title = ir.url_title get :show, :url_title => title assigns[:info_request].attention_requested.should == false - + post :report_request, :url_title => title response.should redirect_to(:action => :show, :url_title => title) - + get :show, :url_title => title response.should be_success assigns[:info_request].attention_requested.should == true @@ -1883,13 +1948,13 @@ describe RequestController, "when reporting a request (logged in)" do it "should not allow a request to be reported twice" do title = info_requests(:badger_request).url_title - + post :report_request, :url_title => title response.should redirect_to(:action => :show, :url_title => title) get :show, :url_title => title response.should be_success response.body.should include("has been reported") - + post :report_request, :url_title => title response.should redirect_to(:action => :show, :url_title => title) get :show, :url_title => title @@ -1901,14 +1966,14 @@ describe RequestController, "when reporting a request (logged in)" do title = info_requests(:badger_request).url_title get :show, :url_title => title response.body.should include("Offensive?") - + post :report_request, :url_title => title response.should redirect_to(:action => :show, :url_title => title) - + get :show, :url_title => title - response.body.should_not include("Offensive?") + response.body.should_not include("Offensive?") response.body.should include("This request has been reported") - + info_requests(:badger_request).set_described_state("successful") get :show, :url_title => title response.body.should_not include("This request has been reported") diff --git a/spec/controllers/track_controller_spec.rb b/spec/controllers/track_controller_spec.rb index 1d38b3055..7daa23769 100644 --- a/spec/controllers/track_controller_spec.rb +++ b/spec/controllers/track_controller_spec.rb @@ -11,7 +11,13 @@ describe TrackController, "when making a new track on a request" do TrackThing.stub!(:create_track_for_request).and_return(@track_thing) TrackThing.stub!(:create_track_for_search_query).and_return(@track_thing) TrackThing.stub!(:find_by_existing_track).and_return(nil) - InfoRequest.stub!(:find_by_url_title).and_return(@ir) + InfoRequest.stub!(:find_by_url_title!) do |url_title| + if url_title == "myrequest" + @ir + else + raise ActiveRecord::RecordNotFound.new("Not found") + end + end @user = mock_model(User) User.stub!(:find).and_return(@user) @@ -32,6 +38,13 @@ describe TrackController, "when making a new track on a request" do get :track_request, :url_title => @ir.url_title, :feed => 'track' response.should redirect_to(:controller => 'request', :action => 'show', :url_title => @ir.url_title) end + + it "should 404 for non-existent requests" do + session[:user_id] = @user.id + lambda { + get :track_request, :url_title => "hjksfdhjk_louytu_qqxxx", :feed => 'track' + }.should raise_error(ActiveRecord::RecordNotFound) + end it "should save a search track and redirect to the right place" do session[:user_id] = @user.id diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index 2a738fa4c..c55127992 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -353,7 +353,7 @@ describe InfoRequest 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 prominence != 'backpage'", + :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]}) InfoRequest.find_old_unclassified({:conditions => ["prominence != 'backpage'"]}) end @@ -362,7 +362,7 @@ describe InfoRequest 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'", + :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]}) InfoRequest.find_old_unclassified end @@ -396,7 +396,7 @@ describe InfoRequest do end it 'should return true if it is awaiting description, isn\'t the holding pen and hasn\'t had an event in 21 days' do - @info_request.is_old_unclassified?.should be_true + (@info_request.is_external? || @info_request.is_old_unclassified?).should be_true end end @@ -484,4 +484,32 @@ describe InfoRequest do end + describe 'when an instance is asked if all can view it' do + + before do + @info_request = InfoRequest.new + end + + it 'should return true if its prominence is normal' do + @info_request.prominence = 'normal' + @info_request.all_can_view?.should == true + end + + it 'should return true if its prominence is backpage' do + @info_request.prominence = 'backpage' + @info_request.all_can_view?.should == true + end + + it 'should return false if its prominence is hidden' do + @info_request.prominence = 'hidden' + @info_request.all_can_view?.should == false + end + + it 'should return false if its prominence is requester_only' do + @info_request.prominence = 'requester_only' + @info_request.all_can_view?.should == false + end + + end + end diff --git a/spec/models/request_mailer_spec.rb b/spec/models/request_mailer_spec.rb index 64ac35cf7..ea75ec765 100644 --- a/spec/models/request_mailer_spec.rb +++ b/spec/models/request_mailer_spec.rb @@ -223,7 +223,7 @@ describe RequestMailer, "when sending reminders to requesters to classify a resp 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'", + 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"} diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index c11c7c5bc..6a4d0f2d5 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -5,7 +5,7 @@ require File.expand_path(File.join('..', '..', 'config', 'environment'), __FILE_ require 'spec/autorun' require 'spec/rails' -# set a default username and password so we can test +# set a default username and password so we can test config = MySociety::Config.load_default() config['ADMIN_USERNAME'] = 'foo' config['ADMIN_PASSWORD'] = 'baz' @@ -31,7 +31,21 @@ Spec::Runner.configure do |config| # fixture_path must end in a separator config.fixture_path = File.join(Rails.root, 'spec', 'fixtures') + File::SEPARATOR - config.global_fixtures = :users, :public_bodies, :public_body_translations, :public_body_versions, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things, :foi_attachments, :has_tag_string_tags, :holidays, :track_things_sent_emails + config.global_fixtures = :users, + :public_bodies, + :public_body_translations, + :public_body_versions, + :info_requests, + :raw_emails, + :incoming_messages, + :outgoing_messages, + :comments, + :info_request_events, + :track_things, + :foi_attachments, + :has_tag_string_tags, + :holidays, + :track_things_sent_emails # == Fixtures # @@ -66,7 +80,7 @@ Spec::Runner.configure do |config| # For more information take a look at Spec::Runner::Configuration and Spec::Runner end -# XXX No idea what namespace/class/module to put this in +# XXX No idea what namespace/class/module to put this in def receive_incoming_mail(email_name, email_to, email_from = 'geraldinequango@localhost') email_name = file_fixture_name(email_name) content = File.read(email_name) @@ -97,15 +111,15 @@ def rebuild_xapian_index(terms = true, values = true, texts = true, dropfirst = end parse_all_incoming_messages verbose = false - # safe_rebuild=true, which involves forking to avoid memory leaks, doesn't work well with rspec. + # safe_rebuild=true, which involves forking to avoid memory leaks, doesn't work well with rspec. # unsafe is significantly faster, and we can afford possible memory leaks while testing. safe_rebuild = false - ActsAsXapian.rebuild_index(["PublicBody", "User", "InfoRequestEvent"].map{|m| m.constantize}, verbose, terms, values, texts, safe_rebuild) + ActsAsXapian.rebuild_index(["PublicBody", "User", "InfoRequestEvent"].map{|m| m.constantize}, verbose, terms, values, texts, safe_rebuild) end def update_xapian_index verbose = false - ActsAsXapian.update_index(flush_to_disk=false, verbose) + ActsAsXapian.update_index(flush_to_disk=false, verbose) end # Validate an entire HTML page @@ -217,5 +231,5 @@ end class ApplicationController < ActionController::Base def set_popup_banner @popup_banner = nil - end + end end |