From 0f25285ab6f59b42535b21305cfcca00a285f028 Mon Sep 17 00:00:00 2001 From: Shaun McDonald Date: Sun, 12 Apr 2009 23:20:31 +0000 Subject: [PATCH] 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 | 156 ++++++++++------------ app/controllers/user_controller.rb | 3 + db/README | 2 +- test/functional/api_controller_test.rb | 9 +- test/functional/browse_controller_test.rb | 44 +++--- test/functional/user_controller_test.rb | 46 ++++++- test/integration/user_creation_test.rb | 10 ++ test/integration/user_diaries_test.rb | 2 + 8 files changed, 158 insertions(+), 114 deletions(-) create mode 100644 test/integration/user_creation_test.rb diff --git a/app/controllers/browse_controller.rb b/app/controllers/browse_controller.rb index 10145a50a..465184f50 100644 --- a/app/controllers/browse_controller.rb +++ b/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 diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 47c1cff9f..553e841aa 100644 --- a/app/controllers/user_controller.rb +++ b/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 diff --git a/db/README b/db/README index 4f9fac629..39ce18023 100644 --- a/db/README +++ b/db/README @@ -62,7 +62,7 @@ $ psql openstreetmap (This may need authentication or a -u ) > 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 diff --git a/test/functional/api_controller_test.rb b/test/functional/api_controller_test.rb index 32f19265a..9056931b9 100644 --- a/test/functional/api_controller_test.rb +++ b/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 diff --git a/test/functional/browse_controller_test.rb b/test/functional/browse_controller_test.rb index c5a333c0d..f4abd5065 100644 --- a/test/functional/browse_controller_test.rb +++ b/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 diff --git a/test/functional/user_controller_test.rb b/test/functional/user_controller_test.rb index 2278aed0c..d68f1f883 100644 --- a/test/functional/user_controller_test.rb +++ b/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 diff --git a/test/integration/user_creation_test.rb b/test/integration/user_creation_test.rb new file mode 100644 index 000000000..5f664b5c8 --- /dev/null +++ b/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 diff --git a/test/integration/user_diaries_test.rb b/test/integration/user_diaries_test.rb index 2e7a01030..98ea04543 100644 --- a/test/integration/user_diaries_test.rb +++ b/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 -- 2.39.5