diff options
| -rw-r--r-- | app/controllers/body_controller.rb | 4 | ||||
| -rw-r--r-- | app/models/public_body.rb | 10 | ||||
| -rw-r--r-- | spec/models/public_body_spec.rb | 25 |
3 files changed, 30 insertions, 9 deletions
diff --git a/app/controllers/body_controller.rb b/app/controllers/body_controller.rb index 7fb649842..cd72356d1 100644 --- a/app/controllers/body_controller.rb +++ b/app/controllers/body_controller.rb @@ -4,7 +4,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: body_controller.rb,v 1.25 2009-03-24 13:05:01 tony Exp $ +# $Id: body_controller.rb,v 1.26 2009-06-16 22:21:13 francis Exp $ class BodyController < ApplicationController # XXX tidy this up with better error messages, and a more standard infrastructure for the redirect to canonical URL @@ -14,7 +14,7 @@ class BodyController < ApplicationController return end - @public_body = PublicBody.find_by_urlname(params[:url_name]) + @public_body = PublicBody.find_by_url_name_with_historic(params[:url_name]) raise "None found" if @public_body.nil? # XXX proper 404 # If found by historic name, redirect to new name diff --git a/app/models/public_body.rb b/app/models/public_body.rb index 7eaeeae6a..60f1ed183 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -26,7 +26,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: public_body.rb,v 1.144 2009-06-12 15:40:18 francis Exp $ +# $Id: public_body.rb,v 1.145 2009-06-16 22:21:13 francis Exp $ require 'csv' require 'set' @@ -133,20 +133,22 @@ class PublicBody < ActiveRecord::Base end # like find_by_url_name but also search historic url_name if none found - def self.find_by_urlname(name) + def self.find_by_url_name_with_historic(name) found = PublicBody.find_all_by_url_name(name) return found.first if found.size == 1 # Shouldn't we just make url_name unique? raise "Two bodies with the same URL name: #{name}" if found.size > 1 # If none found, then search the history of short names old = PublicBody::Version.find_all_by_url_name(name) - # :conditions => [ "id in (select public_body_id from public_body_versions where url_name = ?)", name ]) + # Find unique public bodies in it + old = old.map { |x| x.public_body_id } + old = old.uniq # Maybe return the first one, so we show something relevant, # rather than throwing an error? raise "Two bodies with the same historical URL name: #{name}" if old.size > 1 return unless old.size == 1 # does acts_as_versioned provide a method that returns the current version? - return PublicBody.find(old.first.public_body_id) + return PublicBody.find(old.first) end diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb index baec4b1da..741b45f89 100644 --- a/spec/models/public_body_spec.rb +++ b/spec/models/public_body_spec.rb @@ -33,20 +33,39 @@ describe PublicBody, "when searching" do fixtures :public_bodies, :public_body_versions it "should find by existing url name" do - body = PublicBody.find_by_urlname('dfh') + body = PublicBody.find_by_url_name_with_historic('dfh') body.id.should == 3 end it "should find by historic url name" do - body = PublicBody.find_by_urlname('hdink') + body = PublicBody.find_by_url_name_with_historic('hdink') body.id.should == 3 body.class.to_s.should == 'PublicBody' end it "should cope with not finding any" do - body = PublicBody.find_by_urlname('idontexist') + body = PublicBody.find_by_url_name_with_historic('idontexist') body.should be_nil end + it "should cope with duplicate historic names" do + body = PublicBody.find_by_url_name_with_historic('dfh') + + # create history with short name "mouse" twice in it + body.short_name = 'Mouse' + body.url_name.should == 'mouse' + body.save! + body.request_email = 'dummy@localhost' + body.save! + # but a different name now + body.short_name = 'Stilton' + body.url_name.should == 'stilton' + body.save! + + # try and find by it + body = PublicBody.find_by_url_name_with_historic('mouse') + body.id.should == 3 + body.class.to_s.should == 'PublicBody' + end end |