More tests, found a bug in the data browser, no fix yet. Fix db readme. Don't show the signup page to logged in users, by redirecting them to the home page. Remove unnessesary begin blocks in the browse controller.

app/controllers/browse_controller.rb

@@ -10,120 +10,106 @@ class BrowseController < ApplicationController
   
   
   def relation 
-    begin
-      @relation = Relation.find(params[:id])
-     
-      @name = @relation.tags['name'].to_s 
-      if @name.length == 0:
-          @name = "#" + @relation.id.to_s
-      end
-	
-      @title = 'Relation | ' + (@name)
-      @next = Relation.find(:first, :order => "id ASC", :conditions => [ "visible = true AND id > :id", { :id => @relation.id }] ) 
-      @prev = Relation.find(:first, :order => "id DESC", :conditions => [ "visible = true AND id < :id", { :id => @relation.id }] ) 
-    rescue ActiveRecord::RecordNotFound
-      @type = "relation"
-      render :action => "not_found", :status => :not_found
+    @relation = Relation.find(params[:id])
+   
+    @name = @relation.tags['name'].to_s 
+    if @name.length == 0:
+      @name = "#" + @relation.id.to_s
     end
+	
+    @title = 'Relation | ' + (@name)
+    @next = Relation.find(:first, :order => "id ASC", :conditions => [ "visible = true AND id > :id", { :id => @relation.id }] ) 
+    @prev = Relation.find(:first, :order => "id DESC", :conditions => [ "visible = true AND id < :id", { :id => @relation.id }] ) 
+  rescue ActiveRecord::RecordNotFound
+    @type = "relation"
+    render :action => "not_found", :status => :not_found
   end
   
   def relation_history
-    begin
-      @relation = Relation.find(params[:id])
+    @relation = Relation.find(params[:id])
      
-      @name = @relation.tags['name'].to_s 
-      if @name.length == 0:
-          @name = "#" + @relation.id.to_s
-      end
-	
-      @title = 'Relation History | ' + (@name)
-    rescue ActiveRecord::RecordNotFound
-      @type = "relation"
-      render :action => "not_found", :status => :not_found
+    @name = @relation.tags['name'].to_s 
+    if @name.length == 0:
+      @name = "#" + @relation.id.to_s
     end
+	
+    @title = 'Relation History | ' + (@name)
+  rescue ActiveRecord::RecordNotFound
+    @type = "relation"
+    render :action => "not_found", :status => :not_found
   end
   
   def way 
-    begin
-      @way = Way.find(params[:id])
+    @way = Way.find(params[:id])
      
-      @name = @way.tags['name'].to_s 
-      if @name.length == 0:
-          @name = "#" + @way.id.to_s
-      end
-	
-      @title = 'Way | ' + (@name)
-      @next = Way.find(:first, :order => "id ASC", :conditions => [ "visible = true AND id > :id", { :id => @way.id }] ) 
-      @prev = Way.find(:first, :order => "id DESC", :conditions => [ "visible = true AND id < :id", { :id => @way.id }] ) 
-    rescue ActiveRecord::RecordNotFound
-      @type = "way"
-      render :action => "not_found", :status => :not_found
+    @name = @way.tags['name'].to_s 
+    if @name.length == 0:
+      @name = "#" + @way.id.to_s
     end
+	
+    @title = 'Way | ' + (@name)
+    @next = Way.find(:first, :order => "id ASC", :conditions => [ "visible = true AND id > :id", { :id => @way.id }] ) 
+    @prev = Way.find(:first, :order => "id DESC", :conditions => [ "visible = true AND id < :id", { :id => @way.id }] ) 
+  rescue ActiveRecord::RecordNotFound
+    @type = "way"
+    render :action => "not_found", :status => :not_found
   end
   
   def way_history 
-    begin
-      @way = Way.find(params[:id])
+    @way = Way.find(params[:id])
      
-      @name = @way.tags['name'].to_s 
-      if @name.length == 0:
-          @name = "#" + @way.id.to_s
-      end
-	
-      @title = 'Way History | ' + (@name)
-    rescue ActiveRecord::RecordNotFound
-      @type = "way"
-      render :action => "not_found", :status => :not_found
+    @name = @way.tags['name'].to_s 
+    if @name.length == 0:
+      @name = "#" + @way.id.to_s
     end
+	
+    @title = 'Way History | ' + (@name)
+  rescue ActiveRecord::RecordNotFound
+    @type = "way"
+    render :action => "not_found", :status => :not_found
   end
 
   def node 
-    begin
-      @node = Node.find(params[:id])
+    @node = Node.find(params[:id])
      
-      @name = @node.tags_as_hash['name'].to_s 
-      if @name.length == 0:
-          @name = "#" + @node.id.to_s
-      end
-	
-      @title = 'Node | ' + (@name)
-      @next = Node.find(:first, :order => "id ASC", :conditions => [ "visible = true AND id > :id", { :id => @node.id }] ) 
-      @prev = Node.find(:first, :order => "id DESC", :conditions => [ "visible = true AND id < :id", { :id => @node.id }] ) 
-    rescue ActiveRecord::RecordNotFound
-      @type = "node"
-      render :action => "not_found", :status => :not_found
+    @name = @node.tags_as_hash['name'].to_s 
+    if @name.length == 0:
+      @name = "#" + @node.id.to_s
     end
+	
+    @title = 'Node | ' + (@name)
+    @next = Node.find(:first, :order => "id ASC", :conditions => [ "visible = true AND id > :id", { :id => @node.id }] ) 
+    @prev = Node.find(:first, :order => "id DESC", :conditions => [ "visible = true AND id < :id", { :id => @node.id }] ) 
+  rescue ActiveRecord::RecordNotFound
+    @type = "node"
+    render :action => "not_found", :status => :not_found
   end
   
   def node_history 
-    begin
-      @node = Node.find(params[:id])
+    @node = Node.find(params[:id])
      
-      @name = @node.tags_as_hash['name'].to_s 
-      if @name.length == 0:
-          @name = "#" + @node.id.to_s
-      end
-	
-      @title = 'Node History | ' + (@name)
-    rescue ActiveRecord::RecordNotFound
-      @type = "way"
-      render :action => "not_found", :status => :not_found
+    @name = @node.tags_as_hash['name'].to_s 
+    if @name.length == 0:
+      @name = "#" + @node.id.to_s
     end
+	
+    @title = 'Node History | ' + (@name)
+  rescue ActiveRecord::RecordNotFound
+    @type = "way"
+    render :action => "not_found", :status => :not_found
   end
   
   def changeset
-    begin
-      @changeset = Changeset.find(params[:id])
-      @node_pages, @nodes = paginate(:old_nodes, :conditions => {:changeset_id => @changeset.id}, :per_page => 20, :parameter => 'node_page')
-      @way_pages, @ways = paginate(:old_ways, :conditions => {:changeset_id => @changeset.id}, :per_page => 20, :parameter => 'way_page')
-      @relation_pages, @relations = paginate(:old_relations, :conditions => {:changeset_id => @changeset.id}, :per_page => 20, :parameter => 'relation_page')
+    @changeset = Changeset.find(params[:id])
+    @node_pages, @nodes = paginate(:old_nodes, :conditions => {:changeset_id => @changeset.id}, :per_page => 20, :parameter => 'node_page')
+    @way_pages, @ways = paginate(:old_ways, :conditions => {:changeset_id => @changeset.id}, :per_page => 20, :parameter => 'way_page')
+    @relation_pages, @relations = paginate(:old_relations, :conditions => {:changeset_id => @changeset.id}, :per_page => 20, :parameter => 'relation_page')
       
-      @title = "Changeset | #{@changeset.id}"
-      @next = Changeset.find(:first, :order => "id ASC", :conditions => [ "id > :id", { :id => @changeset.id }] ) 
-      @prev = Changeset.find(:first, :order => "id DESC", :conditions => [ "id < :id", { :id => @changeset.id }] ) 
-    rescue ActiveRecord::RecordNotFound
-      @type = "changeset"
-      render :action => "not_found", :status => :not_found
-    end
+    @title = "Changeset | #{@changeset.id}"
+    @next = Changeset.find(:first, :order => "id ASC", :conditions => [ "id > :id", { :id => @changeset.id }] ) 
+    @prev = Changeset.find(:first, :order => "id DESC", :conditions => [ "id < :id", { :id => @changeset.id }] ) 
+  rescue ActiveRecord::RecordNotFound
+    @type = "changeset"
+    render :action => "not_found", :status => :not_found
   end
 end

app/controllers/user_controller.rb

@@ -119,6 +119,9 @@ class UserController < ApplicationController
 
   def new
     @title = 'create account'
+    # The user is logged in already, so don't show them the signup page, instead
+    # send them to the home page
+    redirect_to :controller => 'site', :action => 'index' if session[:user]
   end
 
   def login

db/README

@@ -62,7 +62,7 @@ $ psql openstreetmap
 (This may need authentication or a -u <dbowneruid>)
 
 > CREATE FUNCTION maptile_for_point(int8, int8, int4) RETURNS int4 
-  AS '/path/to/rails-port/db/functions/libpgosm', 'maptile_for_point'
+  AS '/path/to/rails-port/db/functions/libpgosm.so', 'maptile_for_point'
   LANGUAGE C STRICT;
 
 Creating database skeleton tables

test/functional/api_controller_test.rb

@@ -18,10 +18,6 @@ class ApiControllerTest < ActionController::TestCase
     # reall reject it, however this is to test to see if the api changes.
   end
 
-  def basic_authorization(user, pass)
-    @request.env["HTTP_AUTHORIZATION"] = "Basic %s" % Base64.encode64("#{user}:#{pass}")
-  end
-
   # -------------------------------------
   # Test reading a bounding box.
   # -------------------------------------
@@ -151,9 +147,10 @@ class ApiControllerTest < ActionController::TestCase
   #  end
   #end
   
-  # MySQL requires that the C based functions are installed for this test to 
-  # work. More information is available from:
+  # MySQL and Postgres require that the C based functions are installed for 
+  # this test to work. More information is available from:
   # http://wiki.openstreetmap.org/index.php/Rails#Installing_the_quadtile_functions
+  # or by looking at the readme in db/README
   def test_changes_simple
     get :changes
     assert_response :success

test/functional/browse_controller_test.rb

@@ -4,46 +4,52 @@ require 'browse_controller'
 class BrowseControllerTest < ActionController::TestCase
   api_fixtures
 
-  def basic_authorization(user, pass)
-    @request.env["HTTP_AUTHORIZATION"] = "Basic %s" % Base64.encode64("#{user}:#{pass}")
-  end
-
-  def content(c)
-    @request.env["RAW_POST_DATA"] = c.to_s
-  end
-
-  # We need to load the home page, then activate the start rjs method
-  # and finally check that the new panel has loaded.
   def test_start
-  
+    get :start
+    assert_response :success
   end
   
-  # Test reading a relation
   def test_read_relation
-    
+    browse_check 'relation', relations(:visible_relation)
   end
   
   def test_read_relation_history
-    
+    browse_check 'relation_history', relations(:visible_relation)
   end
   
   def test_read_way
-    
+    browse_check 'way', ways(:visible_way)
   end
   
   def test_read_way_history
-    
+    browse_check 'way_history', ways(:visible_way)
   end
   
   def test_read_node
-    
+    browse_check 'node', nodes(:visible_node)
   end
   
   def test_read_node_history
-    
+    browse_check 'node', nodes(:visible_node)
   end
   
   def test_read_changeset
-    
+    browse_check 'changeset', changesets(:normal_user_first_change)
+  end
+  
+  # This is a convenience method for most of the above checks
+  # First we check that when we don't have an id, it will correctly return a 404
+  # then we check that we get the correct 404 when a non-existant id is passed
+  # then we check that it will get a successful response, when we do pass an id
+  def browse_check(type, fixture) 
+    get type
+    assert_response :not_found
+    assert_template 'not_found'
+    get type, {:id => -10} # we won't have an id that's negative
+    assert_response :not_found
+    assert_template 'not_found'
+    get type, {:id => fixture.id}
+    assert_response :success
+    assert_template type
   end
 end

test/functional/user_controller_test.rb

@@ -1,8 +1,48 @@
 require File.dirname(__FILE__) + '/../test_helper'
 
 class UserControllerTest < ActionController::TestCase
-  # Replace this with your real tests.
-  def test_truth
-    assert true
+  fixtures :users
+  
+  # The user creation page loads
+  def test_user_create
+    get :new
+    assert_response :success
+    
+    assert_select "html:root", :count => 1 do
+      assert_select "head", :count => 1 do
+        assert_select "title", :text => /create account/, :count => 1
+      end
+      assert_select "body", :count => 1 do
+        assert_select "div#content", :count => 1 do
+          assert_select "form[action='/user/save'][method=post]", :count => 1 do
+            assert_select "input[id=user_email]", :count => 1
+            assert_select "input[id=user_email_confirmation]", :count => 1
+            assert_select "input[id=user_display_name]", :count => 1
+            assert_select "input[id=user_pass_crypt][type=password]", :count => 1
+            assert_select "input[id=user_pass_crypt_confirmation][type=password]", :count => 1
+            assert_select "input[type=submit][value=Signup]", :count => 1
+          end
+        end
+      end
+    end
+  end
+  
+  # Check that the user account page will display and contains some relevant
+  # information for the user
+  def test_view_user_account
+    get :view
+    assert_response :not_found
+    
+    get :view, {:display_name => "test"}
+    assert_response :success
+  end
+  
+  def test_user_api_details
+    get :api_details
+    assert_response :unauthorized
+    
+    basic_authorization(users(:normal_user).email, "test")
+    get :api_details
+    assert_response :success
   end
 end

test/integration/user_creation_test.rb

@@ -0,0 +1,10 @@
+require 'test_helper'
+
+class UserCreationTest < ActionController::IntegrationTest
+  fixtures :users
+
+  def test_create_user_duplicate
+    get '/user/new'
+    assert_response :success
+  end
+end

test/integration/user_diaries_test.rb

@@ -3,6 +3,8 @@ require File.dirname(__FILE__) + '/../test_helper'
 class UserDiariesTest < ActionController::IntegrationTest
   fixtures :users, :diary_entries
 
+  # Test the creation of a diary entry, making sure that you are redirected to 
+  # login page when not logged in
   def test_showing_create_diary_entry
     get_via_redirect '/user/test/diary/new'
     # We should now be at the login page