diff options
| -rw-r--r-- | app/controllers/admin_request_controller.rb | 2 | ||||
| -rw-r--r-- | app/controllers/comment_controller.rb | 2 | ||||
| -rw-r--r-- | app/controllers/help_controller.rb | 2 | ||||
| -rw-r--r-- | app/controllers/request_controller.rb | 28 | ||||
| -rw-r--r-- | app/controllers/track_controller.rb | 2 | ||||
| -rw-r--r-- | app/models/info_request.rb | 6 | ||||
| -rw-r--r-- | spec/controllers/request_controller_spec.rb | 6 | ||||
| -rw-r--r-- | spec/controllers/track_controller_spec.rb | 15 |
8 files changed, 37 insertions, 26 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 4a1fd9df7..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) @@ -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), @@ -793,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), @@ -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/info_request.rb b/app/models/info_request.rb index ba7e44c08..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 diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 8cadbd238..ba714216f 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -1886,6 +1886,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 |