Login and logout links at top right when you are logged out or logged in.

Redirect the full URI for login, not just the action/controller.
After making request, redirect to the URL for the request with a flash to say it is made.

7 files changed, 103 insertions, 55 deletions

diff --git a/app/controllers/application.rb b/app/controllers/application.rb
index bf78dd0b0..c0f158187 100644
--- a/app/controllers/application.rb
+++ b/app/controllers/application.rb
@@ -6,7 +6,7 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: application.rb,v 1.11 2007-10-09 17:29:43 francis Exp $
+# $Id: application.rb,v 1.12 2007-10-10 16:06:17 francis Exp $
 
 
 class ApplicationController < ActionController::Base
@@ -18,6 +18,11 @@ class ApplicationController < ActionController::Base
 
     # Login form
     def signin
+        # The explict signin link uses this to store where it is to go back to
+        if params[:r]
+            session[:request_uri] = params[:r]
+        end
+
         if not params[:user] 
             # First time page is shown
             render :template => 'user_accounts/signin' and return
@@ -31,7 +36,7 @@ class ApplicationController < ActionController::Base
             if @user
                 # Successful login
                 session[:user] = @user.id
-                redirect_to :action => session[:intended_action], :controller => session[:intended_controller], :post_redirect => 1 and return
+                post_redirect session[:intended_uri], session[:intended_params] and return
             else
                 # Failed to authenticate
                 flash[:error] = "Email or password not correct, please try again"
@@ -58,23 +63,26 @@ class ApplicationController < ActionController::Base
         else
             # New user made, redirect back to where we were
             session[:user] = @user.id
-            redirect_to :action => session[:intended_action], :controller => session[:intended_controller], :post_redirect => 1
+            post_redirect session[:intended_uri], session[:intended_params] and return
         end
     end
 
     # Logout form
     def signout
         session[:user] = nil
-        redirect_to frontpage
+        if params[:r]
+            redirect_to params[:r]
+        else
+            redirect_to :action => "index"
+        end
     end
 
     private
 
     # Check the user is logged in
-    def check_authentication
+    def authenticated?
         unless session[:user]
-            session[:intended_action] = action_name
-            session[:intended_controller] = controller_name
+            session[:intended_uri] = @request.request_uri
             session[:intended_params] = params
             redirect_to :action => "signin"
             return false
@@ -87,11 +95,31 @@ class ApplicationController < ActionController::Base
         return User.find(session[:user])
     end
 
-    # For redirects to POST requests
-    before_filter :post_redirect
-    def post_redirect
+    # Post redirect
+    def post_redirect(uri, params)
+        session[:post_redirect_params] = params
+        # XXX what is built in Ruby URI munging function?
+        if uri.include?("?")
+            uri += "&post_redirect=1"
+        else
+            uri += "?post_redirect=1"
+        end
+        redirect_to uri
+    end
+
+    # Default layout shows user in corner, so needs access to it
+    before_filter :authentication_check
+    def authentication_check
+        if session[:user]
+            @user = authenticated_user
+        end
+    end
+
+    # If we are in a redirect to POST request, then set params
+    before_filter :check_in_post_redirect
+    def check_in_post_redirect
         if params[:post_redirect]
-            params.update(session[:intended_params])
+            params.update(session[:post_redirect_params])
         end
     end
 
diff --git a/app/controllers/new_controller.rb b/app/controllers/new_controller.rb
index 84fb0fb81..1da6f1398 100644
--- a/app/controllers/new_controller.rb
+++ b/app/controllers/new_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: new_controller.rb,v 1.3 2007-10-09 17:29:43 francis Exp $
+# $Id: new_controller.rb,v 1.4 2007-10-10 16:06:17 francis Exp $
 
 class NewController < ApplicationController
     def index
@@ -24,16 +24,18 @@ class NewController < ApplicationController
         # This automatically saves dependent objects, such as @info_request, in the same transaction
         if not @info_request.valid?
             render :action => 'index'
-        elsif check_authentication
+        elsif authenticated?
             @info_request.user = authenticated_user
             @info_request.save
+            flash[:notice] = "Your Freedom of Information request has been created."
+            redirect_to :controller => 'request', :id => @info_request
         end
 
         # Save both models
 #        valid = @info_request.valid? 
 #        valid &&= @outgoing_message.valid? # XXX maybe there is a nicer way of preventing lazy boolean evaluation than this
 #        if valid
-#            if check_authentication
+#            if authenticated?
 #                @info_request.save!
 #                @outgoing_message.save!
 #            end
diff --git a/app/views/layouts/default.rhtml b/app/views/layouts/default.rhtml
index 3fa1884b8..729bb6ec6 100644
--- a/app/views/layouts/default.rhtml
+++ b/app/views/layouts/default.rhtml
@@ -13,6 +13,16 @@
         <li><a href="/new">New Request</a></li>
     </ul>
 
+    <% if not (controller.action_name == 'signin' or controller.action_name == 'signup') %>
+        <div id="logged_in_bar">
+        <% if @user %>
+            Hello, <%=h(@user.name)%>! (<%= link_to "Logout", :action => 'signout', :r => request.request_uri %>)
+        <% else %>
+            Hello! (<%= link_to "Login or register", { :action => 'signin', :r => request.request_uri  } %>)
+        <% end %>
+        </div>
+    <% end %>
+
     <% if flash[:notice] %>
         <div id="notice"><%= flash[:notice] %></div>
     <% end %>
diff --git a/app/views/new/create.rhtml b/app/views/new/create.rhtml
deleted file mode 100644
index 00685d69f..000000000
--- a/app/views/new/create.rhtml
+++ /dev/null
@@ -1,8 +0,0 @@
-<% @title = "New FOI request" %>
-
-<p>Your Freedom of Information request has been created.
-
-<p><%= link_to h(@info_request.title), :controller => 'request', :id => @info_request %>
-
-<p>
-
diff --git a/config/routes.rb b/config/routes.rb
index 037d55f0a..84c14e6b9 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -4,39 +4,40 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: routes.rb,v 1.10 2007-10-09 17:12:12 francis Exp $
+# $Id: routes.rb,v 1.11 2007-10-10 16:06:17 francis Exp $
 
 ActionController::Routing::Routes.draw do |map|
-  # The priority is based upon order of creation: first created -> highest priority.
-  
-  # Sample of regular route:
-  # map.connect 'products/:id', :controller => 'catalog', :action => 'view'
-  # Keep in mind you can assign values other than :controller and :action
-  map.connect "/new/:action", :controller => 'new'
-  map.connect "/list/:action", :controller => 'list'
-  map.connect "/request/:id", :controller => 'request', :action => 'index'
-  map.connect "/user/:name", :controller => 'user', :action => 'index'
-
-  map.connect '/admin/:action', :controller => 'admin', :action => 'index'
-  map.connect '/admin/body/:action/:id', :controller => 'admin_public_body'
-
-  map.connect "/:action/:id", :controller => 'index'
-
-  # Sample of named route:
-  # map.purchase 'products/:id/purchase', :controller => 'catalog', :action => 'purchase'
-  # This route can be invoked with purchase_url(:id => product.id)
-
-  # You can have the root of your site routed by hooking up '' 
-  # -- just remember to delete public/index.html.
-  # map.connect '', :controller => "welcome"
-
-  # Allow downloading Web Service WSDL as a file with an extension
-  # instead of a file named 'wsdl'
-  map.connect ':controller/service.wsdl', :action => 'wsdl'
-
-  # Install the default route as the lowest priority.
-  # FAI: Turned off for now, as to be honest I don't trust it from a security point of view.
-  #map.connect ':controller/:action/:id.:format'
-  #map.connect ':controller/:action/:id'
+    # The priority is based upon order of creation: first created -> highest priority.
+
+    # Sample of regular route:
+    # map.connect 'products/:id', :controller => 'catalog', :action => 'view'
+    # Keep in mind you can assign values other than :controller and :action
+    map.connect "/new/:action", :controller => 'new'
+    map.connect "/list/:action", :controller => 'list'
+    map.connect "/request/:id", :controller => 'request', :action => 'index'
+    map.connect "/user/:name", :controller => 'user', :action => 'index'
+
+    map.connect '/admin/:action', :controller => 'admin', :action => 'index'
+    map.connect '/admin/body/:action/:id', :controller => 'admin_public_body'
+
+    map.connect "/:action/:id", :controller => 'index'
+
+    # Sample of named route:
+    # map.purchase 'products/:id/purchase', :controller => 'catalog', :action => 'purchase'
+    # This route can be invoked with purchase_url(:id => product.id)
+
+    # You can have the root of your site routed by hooking up '' 
+    # -- just remember to delete public/index.html.
+    # map.connect '', :controller => "welcome"
+
+    # Allow downloading Web Service WSDL as a file with an extension
+    # instead of a file named 'wsdl'
+    map.connect ':controller/service.wsdl', :action => 'wsdl'
+
+    # Install the default route as the lowest priority.
+    # FAI: Turned off for now, as to be honest I don't trust it from a security point of view.
+    #map.connect ':controller/:action/:id.:format'
+    #map.connect ':controller/:action/:id'
+    map.connect '/:controller/:action'
 end
 
diff --git a/public/stylesheets/main.css b/public/stylesheets/main.css
index 3b5f145c2..a1943d2a6 100644
--- a/public/stylesheets/main.css
+++ b/public/stylesheets/main.css
@@ -142,6 +142,10 @@ label {
     color: #97E300;
 }
 
+#logged_in_bar {
+    text-align: right;
+}
+
 /* Front page */
 
 #explanation {
diff --git a/todo.txt b/todo.txt
index 8d0947378..251719ff3 100644
--- a/todo.txt
+++ b/todo.txt
@@ -5,12 +5,15 @@ Show that you are logged in in top right corner
 
 Send confirmation email
 
+Send actually FOIFA request
+
 Redirect the front page to the new FOI request page
 Make it say "dear" as default letter
 
 Write some tests (try it their way, at every level)
 
-Rename back links in admin interface
+Go through all controllers and make sure index URL works
+After signin, should go back to full URL with ids etc. as well
 
 Tidying
 =======
@@ -52,6 +55,9 @@ http://www.mysociety.org/moin.cgi/FreedomOfInformation
 Check FOE site lots
 http://community.foe.co.uk/tools/right_to_know/request_generator.html
 
+Look at this basic US site
+http://www.rcfp.org/foi_letter/generate.php
+
 Add spelling checker as in WTT
 
 For grey hints in input fields
@@ -60,8 +66,13 @@ http://pauldowman.com/projects/fieldhints/
 Somehow validate all pages with HTML validate
 
 
+
 Sources of public bodies
 ========================
 
 Schools list: www.edubase.gov.uk
 
+Heather has some for central departments
+
+
+