aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/controllers/body_controller.rb7
-rw-r--r--app/controllers/request_controller.rb32
-rw-r--r--app/controllers/user_controller.rb5
-rw-r--r--app/helpers/link_to_helper.rb10
-rw-r--r--app/models/info_request.rb20
-rw-r--r--app/models/request_mailer.rb4
-rw-r--r--app/views/admin_request/list.rhtml6
-rw-r--r--config/routes.rb4
-rw-r--r--db/migrate/037_add_url_name.rb2
-rw-r--r--db/schema.rb4
-rw-r--r--spec/controllers/request_controller_spec.rb14
-rw-r--r--spec/controllers/user_controller_spec.rb12
-rw-r--r--spec/fixtures/info_requests.yml2
-rw-r--r--spec/fixtures/users.yml4
-rw-r--r--todo.txt6
15 files changed, 85 insertions, 47 deletions
diff --git a/app/controllers/body_controller.rb b/app/controllers/body_controller.rb
index 6ad713455..103b500b8 100644
--- a/app/controllers/body_controller.rb
+++ b/app/controllers/body_controller.rb
@@ -4,11 +4,16 @@
# 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.6 2008-02-27 12:09:03 francis Exp $
+# $Id: body_controller.rb,v 1.7 2008-02-27 13:59:51 francis Exp $
class BodyController < ApplicationController
# XXX tidy this up with better error messages, and a more standard infrastructure for the redirect to canonical URL
def show
+ if MySociety::Format.simplify_url_part(params[:url_name]) != params[:url_name]
+ redirect_to :url_name => MySociety::Format.simplify_url_part(params[:url_name])
+ return
+ end
+
@public_bodies = PublicBody.find(:all,
:conditions => [ "url_name = ?", params[:url_name] ])
if @public_bodies.size > 1
diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 3d7e1c869..d02694183 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -4,12 +4,22 @@
# Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
# Email: francis@mysociety.org; WWW: http://www.mysociety.org/
#
-# $Id: request_controller.rb,v 1.59 2008-02-25 11:17:29 francis Exp $
+# $Id: request_controller.rb,v 1.60 2008-02-27 13:59:51 francis Exp $
class RequestController < ApplicationController
def show
- @info_request = InfoRequest.find(params[:id])
+ # Look up by old style numeric identifiers
+ if params[:url_title].match(/^[0-9]+$/)
+ @info_request = InfoRequest.find(params[:url_title].to_i)
+ redirect_to request_url(@info_request)
+ return
+ end
+
+ # Look up by new style text names
+ @info_request = InfoRequest.find_by_url_title(params[:url_title])
+
+ # Other parameters
@info_request_events = @info_request.info_request_events
@info_request_events.sort! { |a,b| a.created_at <=> b.created_at }
@status = @info_request.calculate_status
@@ -110,7 +120,7 @@ class RequestController < ApplicationController
# XXX send_message needs the database id, so we send after saving, which isn't ideal if the request broke here.
@outgoing_message.send_message
flash[:notice] = "Your Freedom of Information request has been created and sent on its way."
- redirect_to show_request_url(:id => @info_request)
+ redirect_to show_request_url(:url_title => @info_request.url_title)
else
# do nothing - as "authenticated?" has done the redirect to signin page for us
end
@@ -125,7 +135,7 @@ class RequestController < ApplicationController
if !params[:submitted_describe_state].nil?
flash[:notice] = "The status of this request was made up to date elsewhere while you were filling in the form."
end
- redirect_to show_request_url(:id => @info_request)
+ redirect_to show_request_url(:url_title => @info_request.url_title)
return
end
@@ -137,7 +147,7 @@ class RequestController < ApplicationController
if @last_info_request_event_id.nil?
flash[:notice] = "Internal error - awaiting description, but no event to describe"
- redirect_to show_request_url(:id => @info_request)
+ redirect_to show_request_url(:url_title => @info_request.url_title)
return
end
@@ -169,10 +179,10 @@ class RequestController < ApplicationController
# Display appropriate next page (e.g. help for complaint etc.)
if @info_request.calculate_status == 'waiting_response'
flash[:notice] = "<p>Thank you! Hopefully your wait isn't too long.</p> <p>By law, you should get a response before the end of <strong>" + simple_date(@info_request.date_response_required_by) + "</strong>.</p>"
- redirect_to show_request_url(:id => @info_request)
+ redirect_to show_request_url(:url_title => @info_request.url_title)
elsif @info_request.calculate_status == 'waiting_response_overdue'
flash[:notice] = "<p>Thank you! Hope you don't have to wait much longer.</p> <p>By law, you should have got a response before the end of <strong>" + simple_date(@info_request.date_response_required_by) + "</strong>.</p>"
- redirect_to show_request_url(:id => @info_request)
+ redirect_to show_request_url(:url_title => @info_request.url_title)
elsif @info_request.calculate_status == 'rejected'
# XXX explain how to complain
flash[:notice] = "Oh no! Sorry to hear that your request was rejected. Here is what to do now."
@@ -180,17 +190,17 @@ class RequestController < ApplicationController
elsif @info_request.calculate_status == 'successful'
flash[:notice] = "We're glad you got all the information that you wanted. Thank you for using foi.mysociety.org"
# XXX quiz them here for a comment
- redirect_to show_request_url(:id => @info_request)
+ redirect_to show_request_url(:url_title => @info_request.url_title)
elsif @info_request.calculate_status == 'partially_successful'
flash[:notice] = "We're glad you got some of the information that you wanted."
# XXX explain how to complain / quiz them for a comment
- redirect_to show_request_url(:id => @info_request)
+ redirect_to show_request_url(:url_title => @info_request.url_title)
elsif @info_request.calculate_status == 'waiting_clarification'
flash[:notice] = "Please write your follow up message containing the necessary clarifications below."
redirect_to show_response_url(:id => @info_request.id, :incoming_message_id => @events_needing_description[-1].params[:incoming_message_id])
elsif @info_request.calculate_status == 'requires_admin'
flash[:notice] = "Thanks! The foi.mysociety.org team have been notified."
- redirect_to show_request_url(:id => @info_request)
+ redirect_to show_request_url(:url_title => @info_request.url_title)
else
raise "unknown calculate_status " + @info_request.calculate_status
end
@@ -242,7 +252,7 @@ class RequestController < ApplicationController
@outgoing_message.send_message
@outgoing_message.save!
flash[:notice] = "Your follow up message has been created and sent on its way."
- redirect_to show_request_url(:id => @info_request)
+ redirect_to show_request_url(:url_title => @info_request.url_title)
else
# do nothing - as "authenticated?" has done the redirect to signin page for us
end
diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb
index 50f8ece10..94fe731ba 100644
--- a/app/controllers/user_controller.rb
+++ b/app/controllers/user_controller.rb
@@ -4,13 +4,14 @@
# Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
# Email: francis@mysociety.org; WWW: http://www.mysociety.org/
#
-# $Id: user_controller.rb,v 1.29 2008-02-27 12:20:22 francis Exp $
+# $Id: user_controller.rb,v 1.30 2008-02-27 13:59:52 francis Exp $
class UserController < ApplicationController
# Show page about a set of users with same url name
def show
- if MySociety::Format.simplify_url_part(params[:url_name]) != params[:url_name]
+ if MySociety::Format.simplify_url_part(params[:url_name]) != params[:url_name]
redirect_to :url_name => MySociety::Format.simplify_url_part(params[:url_name])
+ return
end
@display_users = User.find(:all, :conditions => [ "url_name = ?", params[:url_name] ], :order => "created_at desc")
diff --git a/app/helpers/link_to_helper.rb b/app/helpers/link_to_helper.rb
index 8d4a1baca..6d57ed353 100644
--- a/app/helpers/link_to_helper.rb
+++ b/app/helpers/link_to_helper.rb
@@ -5,7 +5,7 @@
# Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
# Email: francis@mysociety.org; WWW: http://www.mysociety.org/
#
-# $Id: link_to_helper.rb,v 1.18 2008-02-27 12:18:28 francis Exp $
+# $Id: link_to_helper.rb,v 1.19 2008-02-27 13:59:52 francis Exp $
module LinkToHelper
@@ -13,7 +13,7 @@ module LinkToHelper
# Requests
def request_url(info_request)
- return show_request_url(:id => info_request, :only_path => true)
+ return show_request_url(:url_title => info_request.url_title, :only_path => true)
end
def request_link(info_request)
link_to h(info_request.title), request_url(info_request)
@@ -74,11 +74,7 @@ module LinkToHelper
end
- def info_request_link(info_request)
- link_to h(info_request.title), show_request_url(:id => info_request)
- end
-
-
+ # Admin pages
def admin_url(relative_path)
admin_url_prefix = MySociety::Config.get("ADMIN_BASE_URL", "/admin/")
return admin_url_prefix + relative_path
diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index 72bbf8b6a..c6dc88aea 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -20,12 +20,13 @@
# Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
# Email: francis@mysociety.org; WWW: http://www.mysociety.org/
#
-# $Id: info_request.rb,v 1.47 2008-02-27 12:04:10 francis Exp $
+# $Id: info_request.rb,v 1.48 2008-02-27 13:59:52 francis Exp $
require 'digest/sha1'
class InfoRequest < ActiveRecord::Base
validates_presence_of :title, :message => "^Please enter a summary of your request"
+ validates_format_of :title, :with => /[a-z]/, :message => "^Please write a summary with some text in it", :if => Proc.new { |info_request| !info_request.title.nil? && !info_request.title.empty? }
belongs_to :user
#validates_presence_of :user_id # breaks during construction of new ones :(
@@ -62,6 +63,23 @@ class InfoRequest < ActiveRecord::Base
end
public
+ # When name is changed, also change the url name
+ def title=(title)
+ write_attribute(:title, title)
+ self.update_url_title
+ end
+ def update_url_title
+ url_title = MySociety::Format.simplify_url_part(self.title)
+ if url_title.size > 32
+ url_title = url_title[0..31]
+ end
+ # For request with same name as others, tag on the request numeric id
+ while not InfoRequest.find_by_url_title(url_title).nil?
+ url_title += "-" + self.id.to_s
+ end
+ write_attribute(:url_title, url_title)
+ end
+
# Email which public body should use to respond to request. This is in
# the format PREFIXrequest-ID-HASH@DOMAIN. Here ID is the id of the
# FOI request, and HASH is a signature for that id.
diff --git a/app/models/request_mailer.rb b/app/models/request_mailer.rb
index 20ac78b70..d4f85659b 100644
--- a/app/models/request_mailer.rb
+++ b/app/models/request_mailer.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: request_mailer.rb,v 1.23 2008-02-22 01:58:36 francis Exp $
+# $Id: request_mailer.rb,v 1.24 2008-02-27 13:59:52 francis Exp $
class RequestMailer < ApplicationMailer
@@ -55,7 +55,7 @@ class RequestMailer < ApplicationMailer
@recipients = @from
@subject = "Unusual FOI response, requires admin attention"
# XXX these are repeats of things in helpers/link_to_helper.rb, and shouldn't be
- url = show_request_url(:id => info_request)
+ url = show_request_url(:url_title => info_request.url_title)
admin_url = MySociety::Config.get("ADMIN_BASE_URL", "/admin/") + 'request/show/' + info_request.id.to_s
@body = {:info_request => info_request, :url => url, :admin_url => admin_url }
end
diff --git a/app/views/admin_request/list.rhtml b/app/views/admin_request/list.rhtml
index 9472ac8c1..0ffd07d0e 100644
--- a/app/views/admin_request/list.rhtml
+++ b/app/views/admin_request/list.rhtml
@@ -4,15 +4,15 @@
<table>
<tr>
- <% for column in InfoRequest.content_columns %>
- <th><%= column.human_name %></th>
+ <% for column in InfoRequest.content_columns.map { |c| c.human_name } - [ "Url title" ] %>
+ <th><%= column %></th>
<% end %>
</tr>
<% for info_request in @info_requests %>
<tr class="<%= cycle('odd', 'even') %>">
<td><%= link_to h(info_request.title), 'show/' + info_request.id.to_s %></td>
- <% for column in InfoRequest.content_columns.map { |c| c.name } - [ "title" ] %>
+ <% for column in InfoRequest.content_columns.map { |c| c.name } - [ "title", "url_title" ] %>
<td><%=h info_request.send(column) %></td>
<% end %>
</tr>
diff --git a/config/routes.rb b/config/routes.rb
index cc5032f09..17124bed2 100644
--- a/config/routes.rb
+++ b/config/routes.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: routes.rb,v 1.39 2008-02-27 12:18:29 francis Exp $
+# $Id: routes.rb,v 1.40 2008-02-27 13:59:52 francis Exp $
ActionController::Routing::Routes.draw do |map|
# The priority is based upon order of creation: first created -> highest priority.
@@ -20,7 +20,7 @@ ActionController::Routing::Routes.draw do |map|
request.request_list '/list', :action => 'list'
request.new_request '/new', :action => 'new'
request.new_request_to_body '/new/:public_body_id', :action => 'new'
- request.show_request '/request/:id', :action => 'show'
+ request.show_request '/request/:url_title', :action => 'show'
request.describe_state '/request/:id/describe', :action => 'describe_state'
request.show_response_no_followup '/request/:id/response', :action => 'show_response'
request.show_response '/request/:id/response/:incoming_message_id', :action => 'show_response'
diff --git a/db/migrate/037_add_url_name.rb b/db/migrate/037_add_url_name.rb
index dabb4889f..0a667e0d3 100644
--- a/db/migrate/037_add_url_name.rb
+++ b/db/migrate/037_add_url_name.rb
@@ -13,6 +13,6 @@ class AddUrlName < ActiveRecord::Migration
def self.down
remove_column :public_bodies, :url_name
- remove_column :public_body_versions, :url_name, :text
+ remove_column :public_body_versions, :url_name
end
end
diff --git a/db/schema.rb b/db/schema.rb
index 08ecdcce6..0e8c26f80 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -9,7 +9,7 @@
#
# It's strongly recommended to check this file into your version control system.
-ActiveRecord::Schema.define(:version => 38) do
+ActiveRecord::Schema.define(:version => 39) do
create_table "incoming_messages", :force => true do |t|
t.integer "info_request_id", :null => false
@@ -36,10 +36,12 @@ ActiveRecord::Schema.define(:version => 38) do
t.string "described_state", :null => false
t.boolean "awaiting_description", :default => false, :null => false
t.string "prominence", :default => "normal", :null => false
+ t.text "url_title", :null => false
end
add_index "info_requests", ["created_at"], :name => "index_info_requests_on_created_at"
add_index "info_requests", ["title"], :name => "index_info_requests_on_title"
+ add_index "info_requests", ["url_title"], :name => "index_info_requests_on_url_title", :unique => true
create_table "outgoing_messages", :force => true do |t|
t.integer "info_request_id", :null => false
diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb
index 22f354cae..4d68c2b8c 100644
--- a/spec/controllers/request_controller_spec.rb
+++ b/spec/controllers/request_controller_spec.rb
@@ -53,28 +53,28 @@ describe RequestController, "when showing one request" do
fixtures :info_requests, :info_request_events, :public_bodies, :users, :incoming_messages, :outgoing_messages # all needed as integrating views
it "should be successful" do
- get :show, :id => 101
+ get :show, :url_title => 'why_do_you_have_such_a_fancy_dog'
response.should be_success
end
it "should render with 'show' template" do
- get :show, :id => 101
+ get :show, :url_title => 'why_do_you_have_such_a_fancy_dog'
response.should render_template('show')
end
it "should assign the request" do
- get :show, :id => 101
+ get :show, :url_title => 'why_do_you_have_such_a_fancy_dog'
assigns[:info_request].should == info_requests(:fancy_dog_request)
end
it "should show incoming messages" do
- get :show, :id => 101
+ get :show, :url_title => 'why_do_you_have_such_a_fancy_dog'
size_before = assigns[:info_request_events].size
ir = info_requests(:fancy_dog_request)
receive_incoming_mail('incoming-request-plain.email', ir.incoming_email)
- get :show, :id => 101
+ get :show, :url_title => 'why_do_you_have_such_a_fancy_dog'
(assigns[:info_request_events].size - size_before).should == 1
end
end
@@ -141,7 +141,7 @@ describe RequestController, "when creating a new request" do
mail = deliveries[0]
mail.body.should =~ /This is a silly letter. It is too short to be interesting./
- response.should redirect_to(:controller => 'request', :action => 'show', :id => ir.id)
+ response.should redirect_to(:controller => 'request', :action => 'show', :url_title => ir.url_title)
end
it "should give an error if the same request is submitted twice" do
@@ -227,7 +227,7 @@ describe RequestController, "when sending a followup message" do
mail.body.should =~ /What a useless response! You suck./
mail.to_addrs.to_s.should == "FOI Person <foiperson@localhost>"
- response.should redirect_to(:controller => 'request', :action => 'show', :id => info_requests(:fancy_dog_request))
+ response.should redirect_to(:controller => 'request', :action => 'show', :url_title => info_requests(:fancy_dog_request).url_title)
end
diff --git a/spec/controllers/user_controller_spec.rb b/spec/controllers/user_controller_spec.rb
index 331428cf8..3bf7aaf51 100644
--- a/spec/controllers/user_controller_spec.rb
+++ b/spec/controllers/user_controller_spec.rb
@@ -5,27 +5,27 @@ describe UserController, "when showing a user" do
fixtures :users, :outgoing_messages, :incoming_messages, :info_requests, :info_request_events
it "should be successful" do
- get :show, :url_name => "bob-smith"
+ get :show, :url_name => "bob_smith"
response.should be_success
end
it "should redirect to lower case name if given one with capital letters" do
- get :show, :url_name => "Bob-Smith"
- response.should redirect_to(:controller => 'user', :action => 'show', :url_name => "bob-smith")
+ get :show, :url_name => "Bob_Smith"
+ response.should redirect_to(:controller => 'user', :action => 'show', :url_name => "bob_smith")
end
it "should render with 'show' template" do
- get :show, :url_name => "bob-smith"
+ get :show, :url_name => "bob_smith"
response.should render_template('show')
end
it "should assign the user" do
- get :show, :url_name => "bob-smith"
+ get :show, :url_name => "bob_smith"
assigns[:display_users].should == [ users(:bob_smith_user) ]
end
it "should assign the user for a more complex name" do
- get :show, :url_name => "silly-emnameem"
+ get :show, :url_name => "silly_emnameem"
assigns[:display_users].should == [ users(:silly_name_user) ]
end
diff --git a/spec/fixtures/info_requests.yml b/spec/fixtures/info_requests.yml
index d9aa0d627..a849542b8 100644
--- a/spec/fixtures/info_requests.yml
+++ b/spec/fixtures/info_requests.yml
@@ -1,6 +1,7 @@
fancy_dog_request:
id: 101
title: Why do you have such a fancy dog?
+ url_title: why_do_you_have_such_a_fancy_dog
created_at: 2007-10-11 18:15:57
updated_at: 2007-10-11 18:15:57
public_body_id: 2
@@ -10,6 +11,7 @@ fancy_dog_request:
naughty_chicken_request:
id: 103
title: How much public money is wasted on breeding naughty chickens?
+ url_title: how_much_public_money_is_wasted_o
created_at: 2007-10-13 18:15:57
updated_at: 2007-10-13 18:15:57
public_body_id: 2
diff --git a/spec/fixtures/users.yml b/spec/fixtures/users.yml
index 9daa34bd0..f7ae3ba62 100644
--- a/spec/fixtures/users.yml
+++ b/spec/fixtures/users.yml
@@ -1,7 +1,7 @@
bob_smith_user:
id: "1"
name: Bob Smith
- url_name: bob-smith
+ url_name: bob_smith
email: bob@localhost
salt: "-6116981980.392287733335677"
hashed_password: 6b7cd45a5f35fd83febc0452a799530398bfb6e8 # jonespassword
@@ -11,7 +11,7 @@ bob_smith_user:
silly_name_user:
id: "2"
name: "Silly <em>Name</em>"
- url_name: silly-emnameem
+ url_name: silly_emnameem
email: silly@localhost
salt: "-6116981980.392287733335677"
hashed_password: 6b7cd45a5f35fd83febc0452a799530398bfb6e8 # jonespassword
diff --git a/todo.txt b/todo.txt
index 1ec42df8a..2b85a0516 100644
--- a/todo.txt
+++ b/todo.txt
@@ -1,5 +1,7 @@
-and request!
+This is knackered:
+http://foi.mysociety.org/request/14/response/44/attach/3/Marie's%20letter.tif
+Load errors
FOI requests to use to test it
==============================
@@ -16,6 +18,8 @@ BAILII - relationship with law courts, robots.txt ?
Next
====
+Update test code
+
Short name for highways agency
Search and replace text "FOI" and "Freedom of Information" out the way more
generated by cgit v1.2.3 (git 2.25.1) at 2025-07-22 18:56:47 +0000