aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/controllers/admin_request_controller.rb2
-rw-r--r--app/controllers/comment_controller.rb2
-rw-r--r--app/controllers/help_controller.rb2
-rw-r--r--app/controllers/request_controller.rb28
-rw-r--r--app/controllers/track_controller.rb2
-rw-r--r--app/models/exim_log.rb2
-rw-r--r--app/models/info_request.rb13
-rw-r--r--app/views/request/_correspondence.rhtml2
-rw-r--r--config/routes.rb2
-rw-r--r--lib/quiet_opener.rb4
-rw-r--r--spec/controllers/request_controller_spec.rb6
-rw-r--r--spec/controllers/track_controller_spec.rb15
-rw-r--r--spec/models/info_request_spec.rb6
-rw-r--r--spec/models/request_mailer_spec.rb2
14 files changed, 50 insertions, 38 deletions
diff --git a/app/controllers/admin_request_controller.rb b/app/controllers/admin_request_controller.rb
index 0b58a95e5..ecbc35e1e 100644
--- a/app/controllers/admin_request_controller.rb
+++ b/app/controllers/admin_request_controller.rb
@@ -185,7 +185,7 @@ class AdminRequestController < AdminController
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 + "'"
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 2f60c2fb3..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)
@@ -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
@@ -667,7 +660,7 @@ class RequestController < ApplicationController
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),
@@ -850,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/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/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/info_request.rb b/app/models/info_request.rb
index 2f4a89d91..23e18b858 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'
@@ -565,7 +567,7 @@ public
self.calculate_event_states
- if self.requires_admin?
+ if self.requires_admin? && !self.is_external?
RequestMailer.deliver_requires_admin(self, set_by)
end
end
@@ -942,7 +944,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 +962,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
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/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/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/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb
index 13dbe4b74..f91f87323 100644
--- a/spec/controllers/request_controller_spec.rb
+++ b/spec/controllers/request_controller_spec.rb
@@ -1890,6 +1890,12 @@ describe RequestController, "when reporting a request (logged in)" 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)
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 98ee34381..41d01c89a 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
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"}
generated by cgit v1.2.3 (git 2.25.1) at 2025-07-27 05:11:02 +0000