diff options
48 files changed, 656 insertions, 170 deletions
diff --git a/app/controllers/admin_user_controller.rb b/app/controllers/admin_user_controller.rb index 5d90e74fe..12b4e553f 100644 --- a/app/controllers/admin_user_controller.rb +++ b/app/controllers/admin_user_controller.rb @@ -45,6 +45,7 @@ class AdminUserController < AdminController @admin_user.admin_level = params[:admin_user][:admin_level] @admin_user.ban_text = params[:admin_user][:ban_text] @admin_user.about_me = params[:admin_user][:about_me] + @admin_user.no_limit = params[:admin_user][:no_limit] if @admin_user.valid? @admin_user.save! diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 8fc6c3792..1849f23f3 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -178,14 +178,13 @@ class ApplicationController < ActionController::Base end def foi_fragment_cache_path(param) - path = foi_fragment_cache_part_path(param) - path = "/views" + path - foi_cache_path = File.join(File.dirname(__FILE__), '../../cache') + path = File.join(RAILS_ROOT, 'cache', 'views', foi_fragment_cache_part_path(param)) max_file_length = 255 - 35 # we subtract 35 because tempfile # adds on a variable number of # characters - return File.join(foi_cache_path, path)[0...max_file_length] + return File.join(File.split(path).map{|x| x[0...max_file_length]}) end + def foi_fragment_cache_all_for_request(info_request) # return stub path so admin can expire it first_three_digits = info_request.id.to_s()[0..2] @@ -355,9 +354,7 @@ class ApplicationController < ActionController::Base @sortby = sortby # Work out sorting method - order_pair = order_to_sort_by(@sortby) - order = order_pair[0] - ascending = order_pair[1] + order, ascending = order_to_sort_by(@sortby) # Peform the search @per_page = per_page diff --git a/app/controllers/general_controller.rb b/app/controllers/general_controller.rb index 8ac41b05a..82b1b8629 100644 --- a/app/controllers/general_controller.rb +++ b/app/controllers/general_controller.rb @@ -45,20 +45,21 @@ class GeneralController < ApplicationController :joins => :translations) end end - # Get some successful requests # + # Get some successful requests begin query = 'variety:response (status:successful OR status:partially_successful)' - # query = 'variety:response' # XXX debug - sortby = "described" + sortby = "newest" max_count = 5 xapian_object = perform_search([InfoRequestEvent], query, sortby, 'request_title_collapse', max_count) @request_events = xapian_object.results.map { |r| r[:model] } - @request_events = @request_events.sort_by { |e| e.described_at }.reverse + + # If there are not yet enough successful requests, fill out the list with + # other requests if @request_events.count < max_count query = 'variety:sent' xapian_object = perform_search([InfoRequestEvent], query, sortby, 'request_title_collapse', max_count-@request_events.count) more_events = xapian_object.results.map { |r| r[:model] } - @request_events += more_events.sort_by { |e| e.described_at }.reverse + @request_events += more_events end rescue @request_events = [] diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index a70e8d16c..fc1ffdd75 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -208,8 +208,12 @@ class RequestController < ApplicationController # Banned from making new requests? if !authenticated_user.nil? && !authenticated_user.can_file_requests? - @details = authenticated_user.can_fail_html - render :template => 'user/banned' + if authenticated_user.exceeded_limit? + render :template => 'user/rate_limited' + else + @details = authenticated_user.can_fail_html + render :template => 'user/banned' + end return end diff --git a/app/models/foi_attachment.rb b/app/models/foi_attachment.rb index 74346227b..da92d1c2d 100644 --- a/app/models/foi_attachment.rb +++ b/app/models/foi_attachment.rb @@ -38,7 +38,11 @@ class FoiAttachment < ActiveRecord::Base BODY_MAX_DELAY = 5 def directory - base_dir = File.join(File.dirname(__FILE__), "../../cache", "attachments_#{ENV['RAILS_ENV']}") + rails_env = ENV['RAILS_ENV'] + if rails_env.nil? || rails_env.empty? + raise "$RAILS_ENV is not set" + end + base_dir = File.join(File.dirname(__FILE__), "../../cache", "attachments_#{rails_env}") return File.join(base_dir, self.hexdigest[0..2]) end diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb index 91f1cf7c0..131970ba6 100644 --- a/app/models/incoming_message.rb +++ b/app/models/incoming_message.rb @@ -127,6 +127,9 @@ class IncomingMessage < ActiveRecord::Base # The following fields may be absent; we treat them as cached # values in case we want to regenerate them (due to mail # parsing bugs, etc). + if self.raw_email.nil? + raise "Incoming message id=#{id} has no raw_email" + end if (!force.nil? || self.last_parsed.nil?) ActiveRecord::Base.transaction do self.extract_attachments! diff --git a/app/models/user.rb b/app/models/user.rb index b6839aa31..2193805ea 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -19,6 +19,7 @@ # locale :string(255) # email_bounced_at :datetime # email_bounce_message :text default(""), not null +# no_limit :boolean default(FALSE), not null # # models/user.rb: @@ -256,7 +257,7 @@ class User < ActiveRecord::Base end def User.owns_every_request?(user) - !user.nil? && user.owns_every_request? + !user.nil? && user.owns_every_request? end # Can the user see every request, even hidden ones? @@ -274,7 +275,18 @@ class User < ActiveRecord::Base end # Various ways the user can be banned, and text to describe it if failed def can_file_requests? - self.ban_text.empty? + self.ban_text.empty? && !self.exceeded_limit? + end + def exceeded_limit? + # Some users have no limit + return false if self.no_limit + + # Has the user issued as many as MAX_REQUESTS_PER_USER_PER_DAY requests in the past 24 hours? + daily_limit = MySociety::Config.get("MAX_REQUESTS_PER_USER_PER_DAY") + return false if daily_limit.nil? + recent_requests = InfoRequest.count(:conditions => ["user_id = ? and created_at > now() - '1 day'::interval", self.id]) + + return (recent_requests >= daily_limit) end def can_make_followup? self.ban_text.empty? @@ -286,7 +298,11 @@ class User < ActiveRecord::Base self.ban_text.empty? end def can_fail_html - text = self.ban_text.strip + if ban_text + text = self.ban_text.strip + else + raise "Unknown reason for ban" + end text = CGI.escapeHTML(text) text = MySociety::Format.make_clickable(text, :contract => 1) text = text.gsub(/\n/, '<br>') diff --git a/app/views/admin_user/_form.rhtml b/app/views/admin_user/_form.rhtml index ba2bd8f8b..be69d9a80 100644 --- a/app/views/admin_user/_form.rhtml +++ b/app/views/admin_user/_form.rhtml @@ -8,10 +8,10 @@ <p><label for="admin_user_email">Email</label> (<strong>you must</strong> first validate this)<br/> <%= text_field 'admin_user', 'email', :size => 60 %></p> -<p><label for="admin_level">Admin level</label> (<strong>none</strong> or <strong>super</strong>; this is for admin features and links which are in the site proper)<br/> +<p><label for="admin_user_admin_level">Admin level</label> (<strong>none</strong> or <strong>super</strong>; this is for admin features and links which are in the site proper)<br/> <%= text_field 'admin_user', 'admin_level', :size => 60 %></p> -<p><label for="ban_text">Ban text</label> <small>(if not blank will stop the +<p><label for="admin_user_ban_text">Ban text</label> <small>(if not blank will stop the user from filing new requests, making annotations or messaging other users; the text is shown in public on the user's page and when they try to do a forbidden action; write in the second person (you); see @@ -19,7 +19,9 @@ <%= text_area 'admin_user', 'ban_text', :cols => 60, :rows => 3 %></p> -<p><label for="about_me">About me</label> (user's own text on their profile, format like comments):<br/> +<p><label for="admin_user_about_me">About me</label> (user's own text on their profile, format like comments):<br/> <%= text_area 'admin_user', 'about_me', :cols => 60, :rows => 3 %></p> +<p><%= check_box 'admin_user', 'no_limit' %> +<label for="admin_user_no_limit">No rate limit</label> (disable the limit on daily requests)</p> diff --git a/app/views/user/rate_limited.rhtml b/app/views/user/rate_limited.rhtml new file mode 100644 index 000000000..d513cec9e --- /dev/null +++ b/app/views/user/rate_limited.rhtml @@ -0,0 +1,5 @@ +<% @title = "Too many requests" %> + +<h1><%=@title%></h1> + +<p><%= _('You have made too many requests today. Please try again tomorrow.')%></p> diff --git a/app/views/user/show.rhtml b/app/views/user/show.rhtml index a4466f5f4..8f1803442 100644 --- a/app/views/user/show.rhtml +++ b/app/views/user/show.rhtml @@ -81,7 +81,7 @@ <div id="user_public_banned"> <p> <strong> - <%= _('This user has been banned from {{site_name}} ', :site_name=>site_name)%> + <%= _('This user has been banned from {{site_name}} ', :site_name=>site_name)%> </strong> </p> <p> @@ -125,17 +125,16 @@ <% if @show_requests %> <div id="user_profile_search"> - <% form_tag(show_user_url, :method => "get", :id=>"search_form") do %> - <div> - <%= text_field_tag(:user_query, params[:user_query]) %> - <% if @is_you %> - <%= submit_tag(_("Search your contributions")) %> - <% else %> - <%= submit_tag(_("Search contributions by this person")) %> - <% end %> - </div> - <% end %> - + <% form_tag(show_user_url, :method => "get", :id=>"search_form") do %> + <div> + <%= text_field_tag(:user_query, params[:user_query]) %> + <% if @is_you %> + <%= submit_tag(_("Search your contributions")) %> + <% else %> + <%= submit_tag(_("Search contributions by this person")) %> + <% end %> + </div> + <% end %> <% if !@xapian_requests.nil? %> <% if @xapian_requests.results.empty? %> @@ -143,16 +142,16 @@ <h2 class="foi_results" id="foi_requests"><%= @is_you ? 'Freedom of Information requests made by you' : 'Freedom of Information requests made by this person' %> <%= @match_phrase %> </h2> <p><%= @is_you ? _('You have made no Freedom of Information requests using this site.') : _('This person has made no Freedom of Information requests using this site.') %> - <%= @page_desc %> + <%= @page_desc %> <% end %> <% else %> <h2 class="foi_results" id="foi_requests"> <%= @is_you ? n_('Your %d Freedom of Information request', 'Your %d Freedom of Information requests', @xapian_requests.matches_estimated.to_s) % @xapian_requests.matches_estimated.to_s : n_('This person\'s %d Freedom of Information request', 'This person\'s %d Freedom of Information requests', @xapian_requests.matches_estimated.to_s) % @xapian_requests.matches_estimated %> <!-- matches_estimated <%=@xapian_requests.matches_estimated%> --> <%= @match_phrase %> - <%= @page_desc %> + <%= @page_desc %> </h2> - + <% for result in @xapian_requests.results %> <%= render :partial => 'request/request_listing_via_event', :locals => { :event => result[:model], :info_request => result[:model].info_request } %> @@ -171,9 +170,9 @@ <% if @xapian_comments.results.empty? %> <% if @page == 1 %> <h2><%= @is_you ? _('Your annotations') : _('This person\'s annotations') %> - <%= @match_phrase %> - </h2> - <p><%= _('None made.')%></p> + <%= @match_phrase %> + </h2> + <p><%= _('None made.')%></p> <% end %> <% else %> <h2 id="annotations"> diff --git a/config/test.yml b/config/test.yml index 693cdd6b8..6a423b47a 100644 --- a/config/test.yml +++ b/config/test.yml @@ -120,3 +120,6 @@ HTML_TO_PDF_COMMAND: /usr/local/bin/wkhtmltopdf-amd64 # Exception notifications EXCEPTION_NOTIFICATIONS_FROM: do-not-reply-to-this-address@example.com EXCEPTION_NOTIFICATIONS_TO: + +MAX_REQUESTS_PER_USER_PER_DAY: 2 + diff --git a/db/migrate/110_add_user_no_limit.rb b/db/migrate/110_add_user_no_limit.rb new file mode 100644 index 000000000..d78a05f75 --- /dev/null +++ b/db/migrate/110_add_user_no_limit.rb @@ -0,0 +1,13 @@ +require 'digest/sha1' + +class AddUserNoLimit < ActiveRecord::Migration + def self.up + add_column :users, :no_limit, :boolean, :default => false, :null => false + end + def self.down + remove_column :users, :no_limit + end +end + + + diff --git a/public/stylesheets/main.css b/public/stylesheets/main.css index cd617c115..b9559ba17 100644 --- a/public/stylesheets/main.css +++ b/public/stylesheets/main.css @@ -24,9 +24,9 @@ position:absolute; left:0; z-index:150; text-align:right; --moz-opacity:0.7px; +-moz-opacity:0.7; filter:alpha(opacity= 70) !important; -opacity:0.7px; +opacity:0.7; width:auto; right:0; top:10px; @@ -1245,7 +1245,7 @@ border-radius:2px; -moz-border-radius:2px; text-shadow:1px 1px 0 #5B841D; font-size:18px; -cursor:hand; +cursor:pointer; padding:5px 6px; } @@ -1481,7 +1481,7 @@ top:2px; border:none; background:#FFF; cursor:pointer; -opacity:1px; +opacity:1; } #ui-datepicker-div .ui-datepicker-next-hover { @@ -1490,7 +1490,7 @@ top:2px; border:none; background:#FFF; cursor:pointer; -opacity:1px; +opacity:1; } #other-country-notice { @@ -1502,7 +1502,7 @@ z-index:999; display:block; position:absolute; top:0; -opacity:0.9px; +opacity:0.9; } p.public-body-name-prefix { @@ -1522,7 +1522,7 @@ text-decoration:none; display:inline-block; border-radius:2px; -moz-border-radius:2px; -cursor:hand; +cursor:pointer; background:url(/images/small-white-cross.png) no-repeat; width:15px; height:15px; @@ -1538,7 +1538,7 @@ position:absolute; text-align:left; background-color:#FFF; z-index:999; -opacity:0.9px; +opacity:0.9; border-radius:6px; -moz-border-radius:6px; border:1px solid #444; @@ -1647,7 +1647,7 @@ display:none; #ui-datepicker-div .ui-datepicker-prev,#ui-datepicker-div .ui-datepicker-next { margin-top:2px; -opacity:0.5px; +opacity:0.5; } div.controller_help dt:hover > a:hover,div.controller_help h1:hover > a:hover,div#help_unhappy h1:hover > a.hover_a:hover,h2 a:hover,.request_listing span.head a:hover,.user_listing span.head a:hover,.body_listing span.head a:hover,.request_listing .requester a,.feed_link a:hover,.act_link a:hover,#header_right > a:hover { diff --git a/script/load-sample-data b/script/load-sample-data index 84b8a28eb..92846ce17 100755 --- a/script/load-sample-data +++ b/script/load-sample-data @@ -4,17 +4,16 @@ # the fact that the fixtures aren't aware of the fact that RawEmails # have a filesystem representation of their contents -LOC=`dirname $0` +LOC=`dirname "$0"` + rake --silent spec:db:fixtures:load + "$LOC/runner" /dev/stdin <<END env = ENV["RAILS_ENV"] -require "spec/spec_helper.rb" # Sets RAILS_ENV to 'test' -ENV["RAILS_ENV"] = env +require "spec/spec_helper.rb" # this sets RAILS_ENV to 'test' +ENV["RAILS_ENV"] = env # so restore to what it was before -RawEmail.all().each do |email| - puts "Writing #{email.filepath}" - email.data = load_file_fixture("useless_raw_email.email") -end +load_raw_emails_data END echo "Loaded fixtures." diff --git a/script/spec-all-pairs b/script/spec-all-pairs index 6f27e6b8e..338f56147 100755 --- a/script/spec-all-pairs +++ b/script/spec-all-pairs @@ -41,6 +41,7 @@ pairs_from_stdin() { do case "$line" in \*\ FAILED:\ *) + spec/*.rb spec/$.rb) line=${line#\* FAILED: } if ! test_pair $line then @@ -63,4 +64,4 @@ then else all_pairs fi -exit $?
\ No newline at end of file +exit $? diff --git a/spec/controllers/admin_request_controller_spec.rb b/spec/controllers/admin_request_controller_spec.rb index 6d3c955bb..5e6f6c2a5 100644 --- a/spec/controllers/admin_request_controller_spec.rb +++ b/spec/controllers/admin_request_controller_spec.rb @@ -6,7 +6,7 @@ describe AdminRequestController, "when administering requests" do before { basic_auth_login @request } before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data @old_filters = ActionController::Routing::Routes.filters ActionController::Routing::Routes.filters = RoutingFilter::Chain.new end @@ -53,7 +53,7 @@ describe AdminRequestController, "when administering the holding pen" do fixtures :users, :public_bodies, :public_body_translations, :public_body_versions, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things before(:each) do basic_auth_login @request - load_raw_emails_data(raw_emails) + load_raw_emails_data @old_filters = ActionController::Routing::Routes.filters ActionController::Routing::Routes.filters = RoutingFilter::Chain.new end diff --git a/spec/controllers/general_controller_spec.rb b/spec/controllers/general_controller_spec.rb index c4fd46c14..9a5421a78 100644 --- a/spec/controllers/general_controller_spec.rb +++ b/spec/controllers/general_controller_spec.rb @@ -34,7 +34,7 @@ describe GeneralController, "when searching" do ] before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data rebuild_xapian_index end @@ -111,13 +111,25 @@ describe GeneralController, "when searching" do describe 'when constructing the list of recent requests' do before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data rebuild_xapian_index end - it 'should list the successful request first' do + it 'should list the newest successful request first' do + # Make sure the newest is listed first even if an older one + # has a newer comment or was reclassified more recently: + # https://github.com/sebbacon/alaveteli/issues/370 + # + # This is a deliberate behaviour change, in that the + # previous behaviour (showing more-recently-reclassified + # requests first) was intentional. get :frontpage - assigns[:request_events].first.info_request.should == info_requests(:boring_request) + assigns[:request_events].first.info_request.should == info_requests(:another_boring_request) + end + + it 'should coalesce duplicate requests' do + get :frontpage + assigns[:request_events].map(&:info_request).select{|x|x.url_title =~ /^spam/}.length.should == 1 end end @@ -125,7 +137,7 @@ describe GeneralController, "when searching" do # rebuild xapian index after fixtures loaded before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data rebuild_xapian_index end @@ -156,21 +168,31 @@ describe GeneralController, "when searching" do it "should filter results based on end of URL being 'all'" do get :search, :combined => ['"bob"', "all"] - assigns[:xapian_requests].results.size.should == 2 - assigns[:xapian_users].results.size.should == 1 - assigns[:xapian_bodies].results.size.should == 0 + assigns[:xapian_requests].results.map{|x| x[:model]}.should =~ [ + info_request_events(:useless_outgoing_message_event), + info_request_events(:silly_outgoing_message_event), + info_request_events(:useful_incoming_message_event), + info_request_events(:another_useful_incoming_message_event), + ] + assigns[:xapian_users].results.map{|x| x[:model]}.should == [users(:bob_smith_user)] + assigns[:xapian_bodies].results.should == [] end it "should filter results based on end of URL being 'users'" do get :search, :combined => ['"bob"', "users"] assigns[:xapian_requests].should == nil - assigns[:xapian_users].results.size.should == 1 + assigns[:xapian_users].results.map{|x| x[:model]}.should == [users(:bob_smith_user)] assigns[:xapian_bodies].should == nil end it "should filter results based on end of URL being 'requests'" do get :search, :combined => ['"bob"', "requests"] - assigns[:xapian_requests].results.size.should == 2 + assigns[:xapian_requests].results.map{|x|x[:model]}.should =~ [ + info_request_events(:useless_outgoing_message_event), + info_request_events(:silly_outgoing_message_event), + info_request_events(:useful_incoming_message_event), + info_request_events(:another_useful_incoming_message_event), + ] assigns[:xapian_users].should == nil assigns[:xapian_bodies].should == nil end @@ -179,7 +201,7 @@ describe GeneralController, "when searching" do get :search, :combined => ['"quango"', "bodies"] assigns[:xapian_requests].should == nil assigns[:xapian_users].should == nil - assigns[:xapian_bodies].results.size.should == 1 + assigns[:xapian_bodies].results.map{|x|x[:model]}.should == [public_bodies(:geraldine_public_body)] end it "should show help when searching for nothing" do diff --git a/spec/controllers/public_body_controller_spec.rb b/spec/controllers/public_body_controller_spec.rb index 4b657849a..4d27ea3f5 100644 --- a/spec/controllers/public_body_controller_spec.rb +++ b/spec/controllers/public_body_controller_spec.rb @@ -7,7 +7,7 @@ describe PublicBodyController, "when showing a body" do fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data rebuild_xapian_index end @@ -105,11 +105,14 @@ describe PublicBodyController, "when listing bodies" do end it "should list bodies in alphabetical order" do + # Note that they are alphabetised by localised name get :list response.should render_template('list') - assigns[:public_bodies].should == PublicBody.all(:order => "name", :conditions => "id <> #{PublicBody.internal_admin_body.id}") + assigns[:public_bodies].should == PublicBody.all( + :conditions => "id <> #{PublicBody.internal_admin_body.id}", + :order => "(select name from public_body_translations where public_body_id=public_bodies.id and locale='en')") assigns[:tag].should == "all" assigns[:description].should == "" end @@ -150,7 +153,6 @@ describe PublicBodyController, "when listing bodies" do get :list response.should render_template('list') assigns[:public_bodies].should =~ PublicBody.all(:conditions => "id <> #{PublicBody.internal_admin_body.id}") - end it "should list a machine tagged thing, should get it in both ways" do @@ -196,6 +198,11 @@ describe PublicBodyController, "when doing type ahead searches" do fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things integrate_views + + before(:each) do + load_raw_emails_data + rebuild_xapian_index + end it "should return nothing for the empty query string" do get :search_typeahead, :query => "" @@ -214,16 +221,16 @@ describe PublicBodyController, "when doing type ahead searches" do it "should return all requests matching any of the given keywords" do get :search_typeahead, :query => "Geraldine Humpadinking" response.should render_template('public_body/_search_ahead') - assigns[:xapian_requests].results.size.should == 2 - assigns[:xapian_requests].results[0][:model].name.should == public_bodies(:humpadink_public_body).name - assigns[:xapian_requests].results[1][:model].name.should == public_bodies(:geraldine_public_body).name + assigns[:xapian_requests].results.map{|x|x[:model]}.should =~ [ + public_bodies(:humpadink_public_body), + public_bodies(:geraldine_public_body), + ] end it "should return requests matching the given keywords in any of their locales" do get :search_typeahead, :query => "baguette" # part of the spanish notes response.should render_template('public_body/_search_ahead') - assigns[:xapian_requests].results.size.should == 1 - assigns[:xapian_requests].results[0][:model].name.should == public_bodies(:humpadink_public_body).name + assigns[:xapian_requests].results.map{|x|x[:model]}.should =~ [public_bodies(:humpadink_public_body)] end it "should not return matches for short words" do diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index a77d1ac27..3b58df869 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 fixtures :users, :public_bodies, :public_body_translations, :public_body_versions, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data rebuild_xapian_index end @@ -39,6 +39,7 @@ describe RequestController, "when listing recent requests" do from info_request_events later_events where later_events.created_at > info_request_events.created_at and later_events.info_request_id = info_request_events.info_request_id + and later_events.described_state is not null ) and info_request_events.described_state in ('successful', 'partially_successful') )") @@ -124,7 +125,7 @@ describe RequestController, "when showing one request" do fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things # all needed as integrating views before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data end it "should be successful" do @@ -208,7 +209,8 @@ describe RequestController, "when showing one request" do it "should download attachments" do ir = info_requests(:fancy_dog_request) - ir.incoming_messages.each { |x| x.parse_raw_email! } + ir.incoming_messages.each { |x| x.parse_raw_email!(true) } + get :show, :url_title => 'why_do_you_have_such_a_fancy_dog' response.content_type.should == "text/html" size_before = assigns[:info_request_events].size @@ -219,10 +221,12 @@ 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 ir.reload - get :get_attachment, :incoming_message_id => ir.incoming_messages[1].id, :id => ir.id, :part => 2, :file_name => ['hello.txt'] + + 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'] + + 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 @@ -318,7 +322,7 @@ describe RequestController, "when showing one request" do receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) lambda { - get :get_attachment, :incoming_message_id => ir.incoming_messages[1].id, :id => ir.id, :part => 2, + get :get_attachment, :incoming_message_id => ir.incoming_messages[1].id, :id => ir.id, :part => 2, :file_name => ['http://trying.to.hack'] }.should raise_error(ActiveRecord::RecordNotFound) end @@ -332,12 +336,16 @@ describe RequestController, "when showing one request" do censor_rule.last_edit_editor = "unknown" censor_rule.last_edit_comment = "none" ir.censor_rules << censor_rule - - receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) - - get :get_attachment, :incoming_message_id => ir.incoming_messages[1].id, :id => ir.id, :part => 2, :file_name => ['hello.txt'] - response.content_type.should == "text/plain" - response.should have_text(/xxxxxx hello/) + + begin + receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) + + 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(/xxxxxx hello/) + ensure + ir.censor_rules.clear + end end it "should censor with rules on the user (rather than the request)" do @@ -350,12 +358,16 @@ describe RequestController, "when showing one request" do censor_rule.last_edit_comment = "none" ir.user.censor_rules << censor_rule - receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) - ir.reload + begin + receive_incoming_mail('incoming-request-two-same-name.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.txt'], :skip_cache => 1 - response.content_type.should == "text/plain" - response.should have_text(/xxxxxx hello/) + 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(/xxxxxx hello/) + ensure + ir.user.censor_rules.clear + end end it "should censor attachment names" do @@ -386,9 +398,12 @@ describe RequestController, "when showing one request" do censor_rule.last_edit_editor = "unknown" censor_rule.last_edit_comment = "none" ir.censor_rules << censor_rule - - get :show, :url_title => 'why_do_you_have_such_a_fancy_dog' - response.body.should have_tag("p.attachment strong", /goodbye.txt/m) + begin + get :show, :url_title => 'why_do_you_have_such_a_fancy_dog' + response.body.should have_tag("p.attachment strong", /goodbye.txt/m) + ensure + ir.censor_rules.clear + end end it "should make a zipfile available, which has a different URL when it changes" do @@ -431,7 +446,7 @@ describe RequestController, "when changing prominence of a request" do fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :info_request_events, :track_things # all needed as integrating views before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data end it "should not show hidden requests" do @@ -497,11 +512,11 @@ describe RequestController, "when changing prominence of a request" do ir.save! receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) - get :get_attachment, :incoming_message_id => ir.incoming_messages[1].id, :id => ir.id, :part => 2 + get :get_attachment, :incoming_message_id => ir.incoming_messages[1].id, :id => ir.id, :part => 2, :skip_cache => 1 response.content_type.should == "text/html" response.should_not have_text(/Second hello/) response.should render_template('request/hidden') - get :get_attachment, :incoming_message_id => ir.incoming_messages[1].id, :id => ir.id, :part => 3 + get :get_attachment, :incoming_message_id => ir.incoming_messages[1].id, :id => ir.id, :part => 3, :skip_cache => 1 response.content_type.should == "text/html" response.should_not have_text(/First hello/) response.should render_template('request/hidden') @@ -687,6 +702,58 @@ 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, + :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, + :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, + :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.) + session[:user_id] = users(:robin_user) + users(:robin_user).no_limit = true + users(:robin_user).save! + + 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, + :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, + :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 redirect_to(:action => 'show', :url_title => 'whats_black_and_white_and_red_al') + end end @@ -732,6 +799,7 @@ describe RequestController, "when making a new request" do it "should fail if user is banned" do @user.stub!(:can_file_requests?).and_return(false) + @user.stub!(:exceeded_limit?).and_return(false) @user.should_receive(:can_fail_html).and_return('FAIL!') session[:user_id] = @user.id get :new, :public_body_id => @body.id @@ -745,7 +813,7 @@ describe RequestController, "when viewing an individual response for reply/follo fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things # all needed as integrating views before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data end it "should ask for login if you are logged in as wrong person" do @@ -795,7 +863,7 @@ describe RequestController, "when classifying an information request" do @dog_request = info_requests(:fancy_dog_request) @dog_request.stub!(:is_old_unclassified?).and_return(false) InfoRequest.stub!(:find).and_return(@dog_request) - load_raw_emails_data(raw_emails) + load_raw_emails_data end def post_status(status) @@ -1132,7 +1200,7 @@ describe RequestController, "when sending a followup message" do fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things # all needed as integrating views before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data end it "should require login" do @@ -1215,7 +1283,7 @@ describe RequestController, "sending overdue request alerts" do fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things # all needed as integrating views before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data end it "should send an overdue alert mail to creators of overdue requests" do @@ -1303,7 +1371,7 @@ describe RequestController, "sending unclassified new response reminder alerts" fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things # all needed as integrating views before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data end it "should send an alert" do @@ -1333,7 +1401,7 @@ describe RequestController, "clarification required alerts" do integrate_views fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things # all needed as integrating views before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data end it "should send an alert" do @@ -1387,7 +1455,7 @@ describe RequestController, "comment alerts" do integrate_views fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things # all needed as integrating views before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data end it "should send an alert (once and once only)" do @@ -1462,7 +1530,7 @@ describe RequestController, "when viewing comments" do integrate_views fixtures :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data end it "should link to the user who submitted it" do @@ -1574,7 +1642,7 @@ describe RequestController, "when showing JSON version for API" do fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data end it "should return data in JSON form" do @@ -1611,9 +1679,11 @@ describe RequestController, "when doing type ahead searches" do it "should return all requests matching any of the given keywords" do get :search_typeahead, :q => "money dog" response.should render_template('request/_search_ahead.rhtml') - assigns[:xapian_requests].results.size.should == 2 - assigns[:xapian_requests].results[0][:model].title.should == info_requests(:fancy_dog_request).title - assigns[:xapian_requests].results[1][:model].title.should == info_requests(:naughty_chicken_request).title + assigns[:xapian_requests].results.map{|x|x[:model].info_request}.should =~ [ + info_requests(:fancy_dog_request), + info_requests(:naughty_chicken_request), + info_requests(:another_boring_request), + ] end it "should not return matches for short words" do diff --git a/spec/controllers/request_game_controller_spec.rb b/spec/controllers/request_game_controller_spec.rb index cc0808ef3..1383554a1 100644 --- a/spec/controllers/request_game_controller_spec.rb +++ b/spec/controllers/request_game_controller_spec.rb @@ -4,7 +4,7 @@ describe RequestGameController, "when playing the game" do fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things # all needed as integrating views before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data end it "should show the game homepage" do diff --git a/spec/controllers/track_controller_spec.rb b/spec/controllers/track_controller_spec.rb index ad4d651cb..509aa6725 100644 --- a/spec/controllers/track_controller_spec.rb +++ b/spec/controllers/track_controller_spec.rb @@ -40,7 +40,7 @@ describe TrackController, "when sending alerts for a track" do include LinkToHelper # for main_url before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data rebuild_xapian_index end @@ -118,7 +118,7 @@ describe TrackController, "when viewing RSS feed for a track" do fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data rebuild_xapian_index end @@ -144,7 +144,7 @@ describe TrackController, "when viewing JSON version of a track feed" do fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data rebuild_xapian_index end diff --git a/spec/controllers/user_controller_spec.rb b/spec/controllers/user_controller_spec.rb index 81d3ff2e5..4e14aeaa3 100644 --- a/spec/controllers/user_controller_spec.rb +++ b/spec/controllers/user_controller_spec.rb @@ -10,7 +10,7 @@ describe UserController, "when showing a user" do integrate_views fixtures :users, :public_bodies, :public_body_translations, :public_body_versions, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data rebuild_xapian_index end @@ -46,10 +46,14 @@ describe UserController, "when showing a user" do it "should search the user's contributions" do get :show, :url_name => "bob_smith" - assigns[:xapian_requests].results.map{|x|x[:model].info_request}.should =~ InfoRequest.all(:conditions => "user_id = #{users(:bob_smith_user).id}") + assigns[:xapian_requests].results.map{|x|x[:model].info_request}.should =~ InfoRequest.all( + :conditions => "user_id = #{users(:bob_smith_user).id}") get :show, :url_name => "bob_smith", :user_query => "money" - assigns[:xapian_requests].results.map{|x|x[:model].info_request}.should == [info_requests(:naughty_chicken_request)] + assigns[:xapian_requests].results.map{|x|x[:model].info_request}.should =~ [ + info_requests(:naughty_chicken_request), + info_requests(:another_boring_request), + ] end it "should not show unconfirmed users" do diff --git a/spec/fixtures/comments.yml b/spec/fixtures/comments.yml index ed1925bfa..b73385a55 100644 --- a/spec/fixtures/comments.yml +++ b/spec/fixtures/comments.yml @@ -8,4 +8,13 @@ silly_comment: user_id: "2" created_at: 2008-08-13 01:25:17.486939 +sensible_comment: + id: 2 + body: This a wise and helpful annotation. + info_request_id: 106 + visible: t + comment_type: request + user_id: 1 + created_at: 2008-08-13 01:25:17.486939 + updated_at: 2008-08-13 01:25:17.486939 diff --git a/spec/fixtures/files/useless_raw_email.email b/spec/fixtures/files/raw_emails/1.email index 2e4585af7..2e4585af7 100644 --- a/spec/fixtures/files/useless_raw_email.email +++ b/spec/fixtures/files/raw_emails/1.email diff --git a/spec/fixtures/files/raw_emails/2.email b/spec/fixtures/files/raw_emails/2.email new file mode 100644 index 000000000..eab0b0f8d --- /dev/null +++ b/spec/fixtures/files/raw_emails/2.email @@ -0,0 +1,20 @@ +From: "FOI Person" <foiperson@localhost> +To: "Bob Smith" <bob@localhost> +Date: Tue, 13 Nov 2007 11:39:55 +0000 +Bcc: +Subject: Re: Your email +Reply-To: +In-Reply-To: <471f1eae5d1cb_7347..fdbe67386164@cat.tmail> +Content-Type: text/plain; charset=utf-8 + +Dear “Bob”, + +In the financial year 2010–2011, this Department spent a +total of nine hundred of your earth pounds on the purchase +and repair of boring equipment. + +Yours most sincerely, + +Quentin Nobble-Boston, +Permanent Under-Secretary, +Department for Humpadinking diff --git a/spec/fixtures/files/raw_emails/3.email b/spec/fixtures/files/raw_emails/3.email new file mode 100644 index 000000000..a6e780fe5 --- /dev/null +++ b/spec/fixtures/files/raw_emails/3.email @@ -0,0 +1,19 @@ +From: "The Minister" <msw@localhost> +To: "Bob Smith" <bob@localhost> +Date: Tue, 13 Nov 2009 11:39:55 +0000 +Bcc: +Subject: Re: Your message +Reply-To: +In-Reply-To: <471f1eae5d1cb_7347..fdbe67386165@cat.tmail> +Content-Type: text/plain; charset=utf-8 + +Dear “Bob”, + +In the financial year 2010–2011, this Ministry spent precisely +no money at all on the purchase or repair of boring equipment. + +Yours most sincerely, + +Martin Kibble-von Scratsching, +Chief Assistant to the Assistant Chief, +Ministry of Silly Walks diff --git a/spec/fixtures/files/raw_emails/4.email b/spec/fixtures/files/raw_emails/4.email new file mode 100644 index 000000000..c778e5ba9 --- /dev/null +++ b/spec/fixtures/files/raw_emails/4.email @@ -0,0 +1,17 @@ +From: "The Minister" <msw@localhost> +To: robin@localhost +Date: Tue, 13 Nov 2008 11:39:55 +0000 +Bcc: +Subject: Re: v1agra +Reply-To: +In-Reply-To: <fdshjksdahhjkfsdahjkfsd@gfh.example.com> +Content-Type: text/plain; charset=utf-8 + +Thank you for your spam, which we have processed with pleasure. Please +accept herewith our order for six packs of the finest cheap v1agra. + +Yours most sincerely, + +Martin Kibble-von Scratsching, +Chief Assistant to the Assistant Chief, +Ministry of Silly Walks diff --git a/spec/fixtures/files/raw_emails/5.email b/spec/fixtures/files/raw_emails/5.email new file mode 100644 index 000000000..16b65610a --- /dev/null +++ b/spec/fixtures/files/raw_emails/5.email @@ -0,0 +1,17 @@ +From: "The Minister" <sensewalk@localhost> +To: robin@localhost +Date: Tue, 13 Nov 2008 12:39:55 +0000 +Bcc: +Subject: Re: v1agra +Reply-To: +In-Reply-To: <fdshjks+hihhjkfsdahjkfsd@gfh.example.com> +Content-Type: text/plain; charset=utf-8 + +Thank you for your spam, which we have processed with pleasure. Please +accept herewith our order for six packs of the finest cheap v1agra. + +Yours most sincerely, + +Martin Scratsching-von Kibble, +Assistant (Second class) to the Chief Assistant to the Assistant Chief, +Ministry of Sensible Walks diff --git a/spec/fixtures/incoming_messages.yml b/spec/fixtures/incoming_messages.yml index 2a5dd872b..fca5c716c 100644 --- a/spec/fixtures/incoming_messages.yml +++ b/spec/fixtures/incoming_messages.yml @@ -11,3 +11,23 @@ useful_incoming_message: raw_email_id: 2 created_at: 2012-01-26 10:19:23 updated_at: 2012-01-26 10:19:23 + +another_useful_incoming_message: + id: 3 + info_request_id: 106 + raw_email_id: 3 + created_at: 2007-11-13 18:09:20 + updated_at: 2007-11-13 18:09:20 + +spam_1_incoming_message: + id: 4 + info_request_id: 107 + raw_email_id: 4 + created_at: 2008-11-13 18:09:20 + updated_at: 2008-11-13 18:09:20 +spam_2_incoming_message: + id: 5 + info_request_id: 108 + raw_email_id: 5 + created_at: 2008-11-13 19:09:20 + updated_at: 2008-11-13 19:09:20 diff --git a/spec/fixtures/info_request_events.yml b/spec/fixtures/info_request_events.yml index b19845345..3266ec634 100644 --- a/spec/fixtures/info_request_events.yml +++ b/spec/fixtures/info_request_events.yml @@ -1,37 +1,37 @@ useless_outgoing_message_event: + id: 900 params_yaml: "--- \n\ :outgoing_message_id: 1\n" - id: 900 info_request_id: 101 event_type: sent created_at: 2007-10-12 01:56:58.586598 described_state: outgoing_message_id: 1 silly_outgoing_message_event: + id: 901 params_yaml: "--- \n\ :outgoing_message_id: 2\n" - id: 901 info_request_id: 103 event_type: sent created_at: 2007-10-14 10:41:12.686264 described_state: outgoing_message_id: 2 useless_incoming_message_event: + id: 902 params_yaml: "--- \n\ :incoming_message_id: 1\n" - id: 902 info_request_id: 101 event_type: response created_at: 2007-11-13 18:09:20.042061 described_state: incoming_message_id: 1 silly_comment_event: + id: 903 params_yaml: "--- \n\ :comment_id: 1\n" incoming_message_id: last_described_at: described_state: - id: "903" info_request_id: 101 comment_id: 1 calculated_state: @@ -39,15 +39,17 @@ silly_comment_event: outgoing_message_id: created_at: 2008-08-12 23:05:12.500942 badger_outgoing_message_event: + id: 904 params_yaml: "--- \n\ :outgoing_message_id: 3\n" - id: 904 info_request_id: 104 event_type: sent created_at: 2011-10-12 01:56:58.586598 described_state: waiting_response calculated_state: waiting_response outgoing_message_id: 3 + +# These in chronological order boring_outgoing_message_event: id: 905 params_yaml: "--- \n\ @@ -58,14 +60,93 @@ boring_outgoing_message_event: created_at: 2006-01-12 01:56:58.586598 described_state: waiting_response calculated_state: waiting_response -useful_incoming_message_event: +useful_incoming_message_event: id: 906 params_yaml: "--- \n\ :incoming_message_id: 2\n" incoming_message_id: 2 info_request_id: 105 event_type: response + created_at: 2007-11-13 18:00:20 + described_state: successful + calculated_state: successful + +another_boring_outgoing_message_event: + id: 907 + params_yaml: "--- \n\ + :outgoing_message_id: 5\n" + outgoing_message_id: 5 + info_request_id: 106 + event_type: sent + created_at: 2006-01-12 01:56:58.586598 + described_state: waiting_response + calculated_state: waiting_response +another_useful_incoming_message_event: + id: 908 + params_yaml: "--- \n\ + :incoming_message_id: 3\n" + incoming_message_id: 3 + info_request_id: 106 + event_type: response created_at: 2007-11-13 18:09:20.042061 described_state: successful calculated_state: successful +another_comment_event: + id: 909 + info_request_id: 105 + comment_id: 2 + params_yaml: "--- \n\ + :comment_id: 2\n" + incoming_message_id: + outgoing_message_id: + last_described_at: + described_state: + calculated_state: + event_type: comment + created_at: 2008-08-12 12:05:12.879634 + + +# The spam requests were both successful +spam_1_outgoing_message_event: + id: 910 + params_yaml: "--- \n\ + :outgoing_message_id: 6\n" + outgoing_message_id: 6 + info_request_id: 107 + event_type: sent + created_at: 2001-01-02 01:23:45.6789100 + described_state: waiting_response + calculated_state: waiting_response +spam_1_incoming_message_event: + id: 911 + params_yaml: "--- \n\ + :incoming_message_id: 4\n" + incoming_message_id: 4 + info_request_id: 107 + event_type: response + created_at: 2001-01-03 01:23:45.6789100 + described_state: successful + calculated_state: successful + +spam_2_outgoing_message_event: + id: 912 + params_yaml: "--- \n\ + :outgoing_message_id: 7\n" + outgoing_message_id: 7 + info_request_id: 108 + event_type: sent + created_at: 2001-01-02 02:23:45.6789100 + described_state: waiting_response + calculated_state: waiting_response +spam_2_incoming_message_event: + id: 913 + params_yaml: "--- \n\ + :incoming_message_id: 5\n" + incoming_message_id: 5 + info_request_id: 108 + event_type: response + created_at: 2001-01-03 02:23:45.6789100 + described_state: successful + calculated_state: successful + diff --git a/spec/fixtures/info_requests.yml b/spec/fixtures/info_requests.yml index 9f767e7f3..33e9a16f2 100644 --- a/spec/fixtures/info_requests.yml +++ b/spec/fixtures/info_requests.yml @@ -35,10 +35,46 @@ boring_request: id: 105 title: The cost of boring url_title: the_cost_of_boring - created_at: 2012-01-26 10:19:23 - updated_at: 2012-01-26 10:19:23 + created_at: 2006-01-12 01:56:58.586598 + updated_at: 2008-08-12 12:05:12.879634 public_body_id: 3 user_id: 1 described_state: successful awaiting_description: false idhash: 173fd003 +another_boring_request: + id: 106 + title: The cost of boring + url_title: the_cost_of_boring_two # Not _2, because we want to avoid the search collapsing these two + created_at: 2006-01-12 01:56:58.586598 + updated_at: 2007-11-13 18:09:20.042061 + public_body_id: 5 + user_id: 1 + described_state: successful + awaiting_description: false + idhash: 173fd004 + +# A pair of identical requests (with url_title differing only in the numeric suffix) +# used to test the request de-duplication features. +spam_1_request: + id: 107 + title: Cheap v1agra + url_title: spam_1 + created_at: 2010-01-01 01:23:45.6789100 + created_at: 2010-01-01 01:23:45.6789100 + public_body_id: 5 + user_id: 5 + described_state: successful + awaiting_description: false + idhash: 173fd005 +spam_2_request: + id: 108 + title: Cheap v1agra + url_title: spam_2 + created_at: 2010-01-01 02:23:45.6789100 + created_at: 2010-01-01 02:23:45.6789100 + public_body_id: 6 + user_id: 5 + described_state: successful + awaiting_description: false + idhash: 173fd005 diff --git a/spec/fixtures/outgoing_messages.yml b/spec/fixtures/outgoing_messages.yml index 3afce92f5..d33ca4292 100644 --- a/spec/fixtures/outgoing_messages.yml +++ b/spec/fixtures/outgoing_messages.yml @@ -54,5 +54,35 @@ boring_outgoing_message: created_at: 2012-01-14 01:56:58.586598 what_doing: normal_sort +another_boring_outgoing_message: + id: 5 + info_request_id: 106 + message_type: initial_request + status: sent + body: "How much was spent on boring equipment in the 2010-2011 financial year?" + last_sent_at: 2006-01-12 01:57:58.586598 + created_at: 2006-01-12 01:56:58.586598 + updated_at: 2006-01-12 01:56:58.586598 + what_doing: normal_sort +spam_1_outgoing_message: + id: 6 + info_request_id: 107 + message_type: initial_request + status: sent + body: "Would you like some cheap v1agra?" + last_sent_at: 2007-01-12 01:57:58.586598 + created_at: 2007-01-12 01:56:58.586598 + updated_at: 2007-01-12 01:56:58.586598 + what_doing: normal_sort +spam_2_outgoing_message: + id: 7 + info_request_id: 108 + message_type: initial_request + status: sent + body: "Would you like some cheap v1agra?" + last_sent_at: 2007-01-12 02:57:58.586598 + created_at: 2007-01-12 02:56:58.586598 + updated_at: 2007-01-12 02:56:58.586598 + what_doing: normal_sort diff --git a/spec/fixtures/public_bodies.yml b/spec/fixtures/public_bodies.yml index 93fd0fb2e..a0893f1e5 100644 --- a/spec/fixtures/public_bodies.yml +++ b/spec/fixtures/public_bodies.yml @@ -16,7 +16,7 @@ humpadink_public_body: updated_at: 2007-10-25 10:51:01.161639 last_edit_comment: Not sure what this new department does. request_email: humpadink-requests@localhost - id: "3" + id: 3 version: "2" last_edit_editor: "francis" short_name: DfH @@ -36,3 +36,29 @@ forlorn_public_body: url_name: lonely created_at: 2011-01-26 14:11:02.12345 notes: A very lonely public body that no one has corresponded with +silly_walks_public_body: + id: 5 + version: 1 + name: "Ministry of Silly Walks" + first_letter: M + updated_at: 2007-10-25 10:51:01.161639 + last_edit_comment: Is a comment really required? + request_email: silly-walks-requests@localhost + last_edit_editor: robin + short_name: MSW + url_name: msw + created_at: 2007-10-25 10:51:01.161639 + notes: You know the one. +sensible_walks_public_body: + id: 6 + version: 1 + name: "Ministry of Sensible Walks" + first_letter: M + request_email: sensible-walks-requests@localhost + short_name: SenseWalk + url_name: sensible_walks + notes: I bet you’ve never heard of it. + updated_at: 2008-10-25 10:51:01.161639 + last_edit_comment: Another stunning innovation from your friendly national government + last_edit_editor: robin + created_at: 2008-10-25 10:51:01.161639 diff --git a/spec/fixtures/public_body_translations.yml b/spec/fixtures/public_body_translations.yml index 08727e45c..cbb55bb0c 100644 --- a/spec/fixtures/public_body_translations.yml +++ b/spec/fixtures/public_body_translations.yml @@ -52,3 +52,25 @@ forlorn_en_public_body_translation: url_name: lonely locale: en notes: A very lonely public body that no one has corresponded with + +silly_walks_en_public_body_translation: + id: 6 + public_body_id: 5 + locale: en + name: "Ministry of Silly Walks" + first_letter: M + request_email: silly-walks-requests@localhost + short_name: MSW + url_name: msw + notes: You know the one. + +sensible_walks_en_public_body_translation: + id: 7 + public_body_id: 6 + locale: en + name: "Ministry of Sensible Walks" + first_letter: M + request_email: sensible-walks-requests@localhost + short_name: SenseWalk + url_name: sensible_walks + notes: I bet you’ve never heard of it. diff --git a/spec/fixtures/raw_emails.yml b/spec/fixtures/raw_emails.yml index 32e039cab..ad2bc0a63 100644 --- a/spec/fixtures/raw_emails.yml +++ b/spec/fixtures/raw_emails.yml @@ -1,5 +1,20 @@ -useless_raw_email: +# The actual email messages are in fixtures/files/raw_emails +# +# Note that the words "money" and "bob" are used in some tests +# of the search functions, so if you use either of these words +# in the email text then some tests will have to be updated. + +useless_raw_email: id: 1 -useful_raw_email: +useful_raw_email: id: 2 + +another_useful_raw_email: + id: 3 + +spam_1_raw_email: + id: 4 + +spam_2_raw_email: + id: 5 diff --git a/spec/fixtures/users.yml b/spec/fixtures/users.yml index 16ffec034..8620fb3de 100644 --- a/spec/fixtures/users.yml +++ b/spec/fixtures/users.yml @@ -53,3 +53,16 @@ unconfirmed_user: admin_level: 'none' ban_text: '' about_me: '' +robin_user: + id: 5 + name: Robin Houston + url_name: robin_houston + email: robin@localhost + salt: "-6116981980.392287733335677" + hashed_password: 6b7cd45a5f35fd83febc0452a799530398bfb6e8 # jonespassword + updated_at: 2012-01-01 10:39:15.491593 + created_at: 2012-01-01 10:39:15.491593 + email_confirmed: true + admin_level: 'none' + ban_text: '' + about_me: 'I am the best' diff --git a/spec/integration/errors_spec.rb b/spec/integration/errors_spec.rb index d03323445..ea9caaf12 100644 --- a/spec/integration/errors_spec.rb +++ b/spec/integration/errors_spec.rb @@ -17,7 +17,7 @@ describe "When rendering errors" do ] before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data ActionController::Base.consider_all_requests_local = false end diff --git a/spec/integration/search_request_spec.rb b/spec/integration/search_request_spec.rb index d0a457923..61f8df313 100644 --- a/spec/integration/search_request_spec.rb +++ b/spec/integration/search_request_spec.rb @@ -17,8 +17,7 @@ describe "When searching" do ] before(:each) do - emails = raw_emails.clone - load_raw_emails_data(emails) + load_raw_emails_data rebuild_xapian_index end @@ -35,21 +34,34 @@ describe "When searching" do end it "should correctly filter searches for requests" do - request_via_redirect("post", "/search/bob/requests") + request_via_redirect("post", "/search/bob/requests") response.body.should_not include("One person found") - response.body.should include("FOI requests 1 to 2 of 2") + n = 4 # The number of requests that contain the word "bob" somewhere + # in the email text. At present this is: + # - fancy_dog_request + # - naughty_chicken_request + # - boring_request + # - another_boring_request + # + # In other words it is all requests made by Bob Smith + # except for badger_request, which he did not sign. + response.body.should include("FOI requests 1 to #{n} of #{n}") end it "should correctly filter searches for users" do - request_via_redirect("post", "/search/bob/users") + request_via_redirect("post", "/search/bob/users") response.body.should include("One person found") - response.body.should_not include("FOI requests 1 to 2 of 2") + response.body.should_not include("FOI requests 1 to") end it "should correctly filter searches for successful requests" do request_via_redirect("post", "/search/requests", :query => "bob", :latest_status => ['successful']) - response.body.should include("no results matching your query") + n = 2 # The number of *successful* requests that contain the word "bob" somewhere + # in the email text. At present this is: + # - boring_request + # - another_boring_request + response.body.should include("FOI requests 1 to #{n} of #{n}") end it "should correctly filter searches for comments" do diff --git a/spec/integration/view_request_spec.rb b/spec/integration/view_request_spec.rb index cf1e4ca6c..0787644ff 100644 --- a/spec/integration/view_request_spec.rb +++ b/spec/integration/view_request_spec.rb @@ -17,8 +17,7 @@ describe "When viewing requests" do ] before(:each) do - emails = raw_emails.clone - load_raw_emails_data(emails) + load_raw_emails_data end it "should not make endlessly recursive JSON <link>s" do diff --git a/spec/models/foi_attachment_spec.rb b/spec/models/foi_attachment_spec.rb index 05c4fc5fd..0d122aa1b 100644 --- a/spec/models/foi_attachment_spec.rb +++ b/spec/models/foi_attachment_spec.rb @@ -4,7 +4,7 @@ describe FoiAttachment, " when calculating due date" do fixtures :incoming_messages, :raw_emails, :public_bodies, :public_body_translations, :info_requests, :users, :foi_attachments before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data end it "sets the body" do diff --git a/spec/models/incoming_message_spec.rb b/spec/models/incoming_message_spec.rb index 7808ef24c..c096e61e0 100644 --- a/spec/models/incoming_message_spec.rb +++ b/spec/models/incoming_message_spec.rb @@ -6,7 +6,7 @@ describe IncomingMessage, " when dealing with incoming mail" do before(:each) do @im = incoming_messages(:useless_incoming_message) - load_raw_emails_data(raw_emails) + load_raw_emails_data end after(:all) do @@ -236,7 +236,7 @@ describe IncomingMessage, " when censoring data" do @censor_rule_2.last_edit_comment = "none" @im.info_request.censor_rules << @censor_rule_2 - load_raw_emails_data(raw_emails) + load_raw_emails_data end it "should do nothing to a JPEG" do @@ -336,7 +336,7 @@ describe IncomingMessage, " when censoring whole users" do @censor_rule_1.last_edit_editor = "unknown" @censor_rule_1.last_edit_comment = "none" @im.info_request.user.censor_rules << @censor_rule_1 - load_raw_emails_data(raw_emails) + load_raw_emails_data end it "should apply censor rules to HTML files" do @@ -357,7 +357,7 @@ describe IncomingMessage, " when uudecoding bad messages" do fixtures :incoming_messages, :raw_emails, :public_bodies, :public_body_translations, :info_requests, :users, :foi_attachments before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data end it "should be able to do it at all" do @@ -401,7 +401,7 @@ describe IncomingMessage, "when messages are attached to messages" do fixtures :incoming_messages, :raw_emails, :public_bodies, :public_body_translations, :info_requests, :users, :foi_attachments before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data end it "should flatten all the attachments out" do @@ -426,7 +426,7 @@ describe IncomingMessage, "when Outlook messages are attached to messages" do fixtures :incoming_messages, :raw_emails, :public_bodies, :public_body_translations, :info_requests, :users, :foi_attachments before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data end it "should flatten all the attachments out" do @@ -449,7 +449,7 @@ describe IncomingMessage, "when TNEF attachments are attached to messages" do fixtures :incoming_messages, :raw_emails, :public_bodies, :public_body_translations, :info_requests, :users, :foi_attachments before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data end it "should flatten all the attachments out" do diff --git a/spec/models/info_request_event_spec.rb b/spec/models/info_request_event_spec.rb index a75f4b232..9a340c125 100644 --- a/spec/models/info_request_event_spec.rb +++ b/spec/models/info_request_event_spec.rb @@ -58,7 +58,7 @@ describe InfoRequestEvent do fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data parse_all_incoming_messages end diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index b1baa66a2..3c8860285 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -7,7 +7,7 @@ describe InfoRequest do before(:each) do @im = incoming_messages(:useless_incoming_message) - load_raw_emails_data(raw_emails) + load_raw_emails_data end it 'should compute a hash' do diff --git a/spec/models/outgoing_mailer_spec.rb b/spec/models/outgoing_mailer_spec.rb index c5fde93fc..9b8fb5f98 100644 --- a/spec/models/outgoing_mailer_spec.rb +++ b/spec/models/outgoing_mailer_spec.rb @@ -6,7 +6,7 @@ describe OutgoingMailer, " when working out follow up addresses" do # mocks. Put parts of the tests in spec/lib/tmail_extensions.rb fixtures :public_bodies, :public_body_translations, :public_body_versions, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data end it "should parse them right" do @@ -77,7 +77,7 @@ describe OutgoingMailer, "when working out follow up subjects" do fixtures :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data end it "should prefix the title with 'Freedom of Information request -' for initial requests" do diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb index 97fa04f93..8d667c395 100644 --- a/spec/models/public_body_spec.rb +++ b/spec/models/public_body_spec.rb @@ -249,7 +249,6 @@ describe PublicBody, " when loading CSV files" do it "should import even if no email is provided" do errors, notes = PublicBody.import_csv("1,aBody", '', 'replace', true, 'someadmin') # true means dry run errors.should == [] - puts "notes = #{notes.inspect}" notes.size.should == 2 notes[0].should == "line 1: creating new authority 'aBody' (locale: en):\n\t{\"name\":\"aBody\"}" notes[1].should =~ /Notes: Some bodies are in database, but not in CSV file:\n( [A-Za-z ]+\n)*You may want to delete them manually.\n/ diff --git a/spec/models/request_mailer_spec.rb b/spec/models/request_mailer_spec.rb index 2888213d7..0dd0b749a 100644 --- a/spec/models/request_mailer_spec.rb +++ b/spec/models/request_mailer_spec.rb @@ -3,7 +3,7 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe RequestMailer, " when receiving incoming mail" do fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data ActionMailer::Base.deliveries = [] end diff --git a/spec/models/xapian_spec.rb b/spec/models/xapian_spec.rb index 6f4f49373..ba06f3eac 100644 --- a/spec/models/xapian_spec.rb +++ b/spec/models/xapian_spec.rb @@ -4,7 +4,7 @@ describe User, " when indexing users with Xapian" do fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data rebuild_xapian_index end @@ -39,7 +39,7 @@ end describe PublicBody, " when indexing public bodies with Xapian" do fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data rebuild_xapian_index end @@ -73,7 +73,7 @@ describe PublicBody, " when indexing requests by body they are to" do fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data rebuild_xapian_index end @@ -129,7 +129,7 @@ end describe User, " when indexing requests by user they are from" do fixtures :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things, :public_bodies, :public_body_versions, :public_body_translations before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data rebuild_xapian_index end @@ -208,7 +208,7 @@ end describe User, " when indexing comments by user they are by" do fixtures :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data rebuild_xapian_index end @@ -244,7 +244,7 @@ end describe InfoRequest, " when indexing requests by their title" do fixtures :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data rebuild_xapian_index end @@ -274,7 +274,7 @@ end describe InfoRequest, " when indexing requests by tag" do fixtures :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data rebuild_xapian_index end @@ -296,7 +296,7 @@ end describe PublicBody, " when indexing authorities by tag" do fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data rebuild_xapian_index end @@ -321,7 +321,7 @@ end describe PublicBody, " when only indexing selected things on a rebuild" do fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things before(:each) do - load_raw_emails_data(raw_emails) + load_raw_emails_data rebuild_xapian_index end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 33a28449e..065d9d080 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -107,7 +107,7 @@ def validate_html(html) File.open(tempfilename, "w+") do |f| f.puts html end - if not system($html_validation_script, tempfilename) + if not system($html_validation_script, *($html_validation_script_options +[tempfilename])) raise "HTML validation error in " + tempfilename + " HTTP status: " + @response.response_code.to_s end File.unlink(tempfilename) @@ -131,6 +131,7 @@ utility_search_path = MySociety::Config.get("UTILITY_SEARCH_PATH", ["/usr/bin", $html_validation_script_found = false utility_search_path.each do |d| $html_validation_script = File.join(d, "validate") + $html_validation_script_options = ["--charset=utf-8"] if File.file? $html_validation_script and File.executable? $html_validation_script $html_validation_script_found = true break @@ -178,13 +179,12 @@ def safe_mock_model(model, args = {}) mock end -def load_raw_emails_data(raw_emails) - raw_email = raw_emails(:useless_raw_email) - begin - raw_email.destroy_file_representation! - rescue Errno::ENOENT +def load_raw_emails_data + raw_emails_yml = File.join(Spec::Runner.configuration.fixture_path, "raw_emails.yml") + for raw_email_id in YAML::load_file(raw_emails_yml).map{|k,v| v["id"]} do + raw_email = RawEmail.find(raw_email_id) + raw_email.data = load_file_fixture("raw_emails/%d.email" % [raw_email_id]) end - raw_email.data = load_file_fixture("useless_raw_email.email") end def parse_all_incoming_messages |