]> git.openstreetmap.org Git - rails.git/commitdiff
Merge remote-tracking branch 'osmlab/hash'
authorTom Hughes <tom@compton.nu>
Sun, 4 Aug 2013 11:38:59 +0000 (12:38 +0100)
committerTom Hughes <tom@compton.nu>
Sun, 4 Aug 2013 11:38:59 +0000 (12:38 +0100)
13 files changed:
app/controllers/node_controller.rb
app/controllers/relation_controller.rb
app/controllers/user_controller.rb
app/controllers/way_controller.rb
app/views/user/new.html.erb
app/views/user/terms.html.erb
config/preinitializer.rb
config/routes.rb
test/functional/node_controller_test.rb
test/functional/relation_controller_test.rb
test/functional/user_controller_test.rb
test/functional/way_controller_test.rb
test/integration/user_creation_test.rb

index a172b2dd79bc698c5981af26cafa7fee4fd5c687..1e34bc10d507c5cec1874137dd7c13fe5431a718 100644 (file)
@@ -64,6 +64,10 @@ class NodeController < ApplicationController
 
   # Dump the details on many nodes whose ids are given in the "nodes" parameter.
   def nodes
+    if not params['nodes']
+      raise OSM::APIBadUserInput.new("The parameter nodes is required, and must be of the form nodes=id[,id[,id...]]")
+    end
+
     ids = params['nodes'].split(',').collect { |n| n.to_i }
 
     if ids.length == 0
index 39021218fb00811e4047f45c1f4c0172e0f9164a..9e7466a4ad4f7506b5d165e4584e33aa5424ff87 100644 (file)
@@ -130,19 +130,23 @@ class RelationController < ApplicationController
   end
 
   def relations
+    if not params['relations']
+      raise OSM::APIBadUserInput.new("The parameter relations is required, and must be of the form relations=id[,id[,id...]]")
+    end
+
     ids = params['relations'].split(',').collect { |w| w.to_i }
 
-    if ids.length > 0
-      doc = OSM::API.new.get_xml_doc
+    if ids.length == 0
+      raise OSM::APIBadUserInput.new("No relations were given to search for")
+    end
 
-      Relation.find(ids).each do |relation|
-        doc.root << relation.to_xml_node
-      end
+    doc = OSM::API.new.get_xml_doc
 
-      render :text => doc.to_s, :content_type => "text/xml"
-    else
-      render :text => "You need to supply a comma separated list of ids.", :status => :bad_request
+    Relation.find(ids).each do |relation|
+      doc.root << relation.to_xml_node
     end
+
+    render :text => doc.to_s, :content_type => "text/xml"
   end
 
   def relations_for_way
index b96b66464445575fabf8dc26436fa9658c185590..57d2f7d132c299f4f323959b491cc8b644b2b597 100644 (file)
@@ -24,63 +24,15 @@ class UserController < ApplicationController
 
     if request.xhr?
       render :partial => "terms"
-    elsif using_open_id?
-      # The redirect from the OpenID provider reenters here
-      # again and we need to pass the parameters through to
-      # the open_id_authentication function
-      @user = session.delete(:new_user)
-
-      openid_verify(nil, @user) do |user,verified_email|
-        user.status = "active" if user.email == verified_email
-      end
-
-      if @user.openid_url.nil? or @user.invalid?
-        render :action => 'new'
-      else
-        session[:new_user] = @user
-        render :action => 'terms'
-      end
-    elsif params[:user] and Acl.no_account_creation(request.remote_ip, params[:user][:email].split("@").last)
-      render :action => 'blocked'
     else
-      session[:referer] = params[:referer]
-
       @title = t 'user.terms.title'
-      @user = User.new(params[:user]) if params[:user]
-
-      if params[:user] and params[:user][:openid_url] and @user.pass_crypt.empty?
-        # We are creating an account with OpenID and no password
-        # was specified so create a random one
-        @user.pass_crypt = SecureRandom.base64(16)
-        @user.pass_crypt_confirmation = @user.pass_crypt
-      end
-
-      if @user
-        @user.status = "pending"
+      @user ||= session[:new_user]
 
-        if @user.invalid?
-          if @user.new_record?
-            # Something is wrong with a new user, so rerender the form
-            render :action => :new
-          else
-            # Error in existing user, so go to account settings
-            flash[:errors] = @user.errors
-            redirect_to :action => :account, :display_name => @user.display_name
-          end
-        elsif @user.terms_agreed?
-          # Already agreed to terms, so just show settings
-          redirect_to :action => :account, :display_name => @user.display_name
-        elsif params[:user] and params[:user][:openid_url] and not params[:user][:openid_url].empty?
-          # Verify OpenID before moving on
-          session[:new_user] = @user
-          openid_verify(params[:user][:openid_url], @user)
-        elsif @user.new_record?
-          # Save the user record
-          session[:new_user] = @user
-        end
-      else
-        # Not logged in, so redirect to the login page
+      if !@user
         redirect_to :action => :login, :referer => request.fullpath
+      elsif @user.terms_agreed?
+        # Already agreed to terms, so just show settings
+        redirect_to :action => :account, :display_name => @user.display_name
       end
     end
   end
@@ -248,7 +200,23 @@ class UserController < ApplicationController
     @title = t 'user.new.title'
     @referer = params[:referer] || session[:referer]
 
-    if @user
+    if using_open_id?
+      # The redirect from the OpenID provider reenters here
+      # again and we need to pass the parameters through to
+      # the open_id_authentication function
+      @user = session.delete(:new_user)
+
+      openid_verify(nil, @user) do |user, verified_email|
+        user.status = "active" if user.email == verified_email
+      end
+
+      if @user.openid_url.nil? or @user.invalid?
+        render :action => 'new'
+      else
+        session[:new_user] = @user
+        redirect_to :action => 'terms'
+      end
+    elsif @user
       # The user is logged in already, so don't show them the signup
       # page, instead send them to the home page
       if @referer
@@ -268,6 +236,38 @@ class UserController < ApplicationController
     end
   end
 
+  def create
+    if params[:user] and Acl.no_account_creation(request.remote_ip, params[:user][:email].split("@").last)
+      render :action => 'blocked'
+
+    else
+      session[:referer] = params[:referer]
+
+      @user = User.new(params[:user])
+      @user.status = "pending"
+
+      if @user.openid_url.present? && @user.pass_crypt.empty?
+        # We are creating an account with OpenID and no password
+        # was specified so create a random one
+        @user.pass_crypt = SecureRandom.base64(16)
+        @user.pass_crypt_confirmation = @user.pass_crypt
+      end
+
+      if @user.invalid?
+        # Something is wrong with a new user, so rerender the form
+        render :action => "new"
+      elsif @user.openid_url.present?
+        # Verify OpenID before moving on
+        session[:new_user] = @user
+        openid_verify(@user.openid_url, @user)
+      else
+        # Save the user record
+        session[:new_user] = @user
+        redirect_to :action => :terms
+      end
+    end
+  end
+
   def login
     if params[:username] or using_open_id?
       session[:remember_me] ||= params[:remember_me]
index 6ea27deafa2a24a2226efaf7deb3cdb30a8e0b9c..2ce49b7e84fd9693216646bd53c5228dc14551bb 100644 (file)
@@ -84,23 +84,23 @@ class WayController < ApplicationController
   end
 
   def ways
-    begin
-      ids = params['ways'].split(',').collect { |w| w.to_i }
-    rescue
-      ids = []
+    if not params['ways']
+      raise OSM::APIBadUserInput.new("The parameter ways is required, and must be of the form ways=id[,id[,id...]]")
     end
 
-    if ids.length > 0
-      doc = OSM::API.new.get_xml_doc
+    ids = params['ways'].split(',').collect { |w| w.to_i }
 
-      Way.find(ids).each do |way|
-        doc.root << way.to_xml_node
-      end
+    if ids.length == 0
+      raise OSM::APIBadUserInput.new("No ways were given to search for")
+    end
 
-      render :text => doc.to_s, :content_type => "text/xml"
-    else
-      render :text => "", :status => :bad_request
+    doc = OSM::API.new.get_xml_doc
+
+    Way.find(ids).each do |way|
+      doc.root << way.to_xml_node
     end
+
+    render :text => doc.to_s, :content_type => "text/xml"
   end
 
   ##
index 52abb33113e3837c6824a2c251556b953c97e5f1..7324ddd29773a8edb0c585afa122511245d1a289 100644 (file)
@@ -6,7 +6,7 @@
 
 <%= error_messages_for 'user' %>
 
-<%= form_for :user, :url => { :action => 'terms' } do %>
+<%= form_for :user, :url => { :action => 'create' } do %>
   <%= hidden_field_tag('referer', h(@referer)) unless @referer.nil? %>
 
   <div id="signupForm" class="standard-form">
index d51e52b5f7898f358a1d491f1f5addaefec6439b..33c528292208fba6dc3db01041d4df658dc69d53 100644 (file)
@@ -25,7 +25,7 @@
       </fieldset>
     <% end %>
   </div> 
-  <div class="legale">
+  <div id="contributorTerms" class="legale">
     <%= render :partial => "terms" %>
   </div>
   
index 1cc3f62c780a37bf64e6920bf1ed1bbf144ee9ca..377e6bad4a6a13ac33095926085eccefccc925fb 100644 (file)
@@ -1,7 +1,7 @@
 require 'yaml'
 
-config = YAML.load_file(File.expand_path("../application.yml", __FILE__))
 env = ENV['RAILS_ENV'] || 'development'
+config = YAML.load_file(File.expand_path(env == "test" ? "../example.application.yml" : "../application.yml", __FILE__))
 
 ENV.each do |key,value|
   if key.match(/^OSM_(.*)$/)
index 6896fcc01f7bc39d3cf51094d61d77f1b2124d5c..bc473a4b0d74d86724add2413aef7d6f05addc63 100644 (file)
@@ -134,7 +134,8 @@ OpenStreetMap::Application.routes.draw do
   match '/key' => 'site#key', :via => :get
   match '/id' => 'site#id', :via => :get
   match '/user/new' => 'user#new', :via => :get
-  match '/user/terms' => 'user#terms', :via => [:get, :post]
+  match '/user/new' => 'user#create', :via => :post
+  match '/user/terms' => 'user#terms', :via => :get
   match '/user/save' => 'user#save', :via => :post
   match '/user/:display_name/confirm/resend' => 'user#confirm_resend', :via => :get
   match '/user/:display_name/confirm' => 'user#confirm', :via => [:get, :post]
index 32667d9c904950a44cf04584b4c390b1ad79cc7e..5c01cba3d08500edcf64e1a92e8915b74bc67761 100644 (file)
@@ -418,6 +418,34 @@ class NodeControllerTest < ActionController::TestCase
     assert_response :success, "a valid update request failed"
   end
 
+  ##
+  # test fetching multiple nodes
+  def test_nodes
+    # check error when no parameter provided
+    get :nodes
+    assert_response :bad_request
+
+    # check error when no parameter value provided
+    get :nodes, :nodes => ""
+    assert_response :bad_request
+
+    # test a working call
+    get :nodes, :nodes => "1,2,4,15,17"
+    assert_response :success
+    assert_select "osm" do
+      assert_select "node", :count => 5
+      assert_select "node[id=1][visible=true]", :count => 1
+      assert_select "node[id=2][visible=false]", :count => 1
+      assert_select "node[id=4][visible=true]", :count => 1
+      assert_select "node[id=15][visible=true]", :count => 1
+      assert_select "node[id=17][visible=false]", :count => 1
+    end
+
+    # check error when a non-existent node is included
+    get :nodes, :nodes => "1,2,4,15,17,400"
+    assert_response :not_found
+  end
+
   ##
   # test adding tags to a node
   def test_duplicate_tags
index 502404ccaa73a94338da754d53eaeea9c6ed93fe..4737fdf51f42d6286f7a4b8006ffa6e5deb9be9a 100644 (file)
@@ -114,6 +114,33 @@ class RelationControllerTest < ActionController::TestCase
     end
   end
 
+  ##
+  # test fetching multiple relations
+  def test_relations
+    # check error when no parameter provided
+    get :relations
+    assert_response :bad_request
+
+    # check error when no parameter value provided
+    get :relations, :relations => ""
+    assert_response :bad_request
+
+    # test a working call
+    get :relations, :relations => "1,2,4,7"
+    assert_response :success
+    assert_select "osm" do
+      assert_select "relation", :count => 4
+      assert_select "relation[id=1][visible=true]", :count => 1
+      assert_select "relation[id=2][visible=false]", :count => 1
+      assert_select "relation[id=4][visible=true]", :count => 1
+      assert_select "relation[id=7][visible=true]", :count => 1
+    end
+
+    # check error when a non-existent relation is included
+    get :relations, :relations => "1,2,4,7,400"
+    assert_response :not_found
+  end
+
   # -------------------------------------
   # Test simple relation creation.
   # -------------------------------------
index 7761c0c7e30f1563fe7ced1cb2890d3bec38a24c..61286b2845e94c660e5bc930f8649e6841aba262 100644 (file)
@@ -55,11 +55,12 @@ class UserControllerTest < ActionController::TestCase
     )
 
     assert_routing(
-      { :path => "/user/terms", :method => :get },
-      { :controller => "user", :action => "terms" }
+      { :path => "/user/new", :method => :post },
+      { :controller => "user", :action => "create" }
     )
+
     assert_routing(
-      { :path => "/user/terms", :method => :post },
+      { :path => "/user/terms", :method => :get },
       { :controller => "user", :action => "terms" }
     )
 
@@ -202,7 +203,7 @@ class UserControllerTest < ActionController::TestCase
       end
       assert_select "body", :count => 1 do
         assert_select "div#content", :count => 1 do
-          assert_select "form[action='/user/terms'][method=post]", :count => 1 do
+          assert_select "form[action='/user/new'][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
@@ -322,6 +323,23 @@ class UserControllerTest < ActionController::TestCase
     assert_select "div#signupForm > fieldset > div.form-row > div.field_with_errors > input#user_display_name"
   end
 
+  def test_user_terms_new_user
+    get :terms, {}, { "new_user" => User.new }
+    assert_response :success
+    assert_template :terms
+  end
+
+  def test_user_terms_seen
+    user = users(:normal_user)
+
+    # Set the username cookie
+    @request.cookies["_osm_username"] = user.display_name
+
+    get :terms, {}, { "user" => user }
+    assert_response :redirect
+    assert_redirected_to :action => :account, :display_name => user.display_name
+  end
+
   def test_user_lost_password
     # Test fetching the lost password page
     get :lost_password
index a4b5a192a1bdafd7ea64e103bd5b5b9dbb516a98..a69612350fbad26804b1ea25658bbe1e57122faa 100644 (file)
@@ -79,6 +79,33 @@ class WayControllerTest < ActionController::TestCase
     end
   end
 
+  ##
+  # test fetching multiple ways
+  def test_ways
+    # check error when no parameter provided
+    get :ways
+    assert_response :bad_request
+
+    # check error when no parameter value provided
+    get :ways, :ways => ""
+    assert_response :bad_request
+
+    # test a working call
+    get :ways, :ways => "1,2,4,6"
+    assert_response :success
+    assert_select "osm" do
+      assert_select "way", :count => 4
+      assert_select "way[id=1][visible=true]", :count => 1
+      assert_select "way[id=2][visible=false]", :count => 1
+      assert_select "way[id=4][visible=true]", :count => 1
+      assert_select "way[id=6][visible=true]", :count => 1
+    end
+
+    # check error when a non-existent way is included
+    get :ways, :ways => "1,2,4,6,400"
+    assert_response :not_found
+  end
+
   # -------------------------------------
   # Test simple way creation.
   # -------------------------------------
index 6e253afade6176951c3bffac53985aed7489b576..c565afd1ed132961f2ae4f49175b3995f7c0d275 100644 (file)
@@ -21,7 +21,7 @@ class UserCreationTest < ActionController::IntegrationTest
       display_name = "#{localer.to_s}_new_tester"
       assert_difference('User.count', 0) do
         assert_difference('ActionMailer::Base.deliveries.size', 0) do
-          post '/user/terms',
+          post '/user/new',
             {:user => { :email => dup_email, :email_confirmation => dup_email, :display_name => display_name, :pass_crypt => "testtest", :pass_crypt_confirmation => "testtest"}},
             {"HTTP_ACCEPT_LANGUAGE" => localer.to_s}
         end
@@ -41,7 +41,7 @@ class UserCreationTest < ActionController::IntegrationTest
       email = "#{locale.to_s}_new_tester"
       assert_difference('User.count', 0) do
         assert_difference('ActionMailer::Base.deliveries.size', 0) do
-          post '/user/terms',
+          post '/user/new',
           {:user => {:email => email, :email_confirmation => email, :display_name => dup_display_name, :pass_crypt => "testtest", :pass_crypt_confirmation => "testtest"}},
           {"HTTP_ACCEPT_LANGUAGE" => locale.to_s}
         end
@@ -61,13 +61,12 @@ class UserCreationTest < ActionController::IntegrationTest
 
       assert_difference('User.count', 0) do
         assert_difference('ActionMailer::Base.deliveries.size', 0) do
-          post "/user/terms",
+          post "/user/new",
             {:user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :pass_crypt => "testtest", :pass_crypt_confirmation => "testtest"}}
           end
       end
 
-      assert_response :success
-      assert_template 'user/terms'
+      assert_redirected_to "/user/terms"
 
       assert_difference('User.count') do
         assert_difference('ActionMailer::Base.deliveries.size', 1) do
@@ -108,10 +107,9 @@ class UserCreationTest < ActionController::IntegrationTest
     referer = "/traces/mine"
     assert_difference('User.count') do
       assert_difference('ActionMailer::Base.deliveries.size', 1) do
-        post "/user/terms",
+        post "/user/new",
         {:user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :pass_crypt => password, :pass_crypt_confirmation => password}, :referer => referer }
-        assert_response :success
-        assert_template 'terms'
+        assert_redirected_to "/user/terms"
         post_via_redirect "/user/save",
         {:user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :pass_crypt => password, :pass_crypt_confirmation => password} }
       end
@@ -154,13 +152,12 @@ class UserCreationTest < ActionController::IntegrationTest
     password = "testtest"
     assert_difference('User.count') do
       assert_difference('ActionMailer::Base.deliveries.size', 1) do
-        post "/user/terms",
+        post "/user/new",
           {:user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :openid_url => "http://localhost:1123/john.doe?openid.success=newuser", :pass_crypt => "", :pass_crypt_confirmation => ""}}
         assert_response :redirect
         res = openid_request(@response.redirect_url)
-        post '/user/terms', res
-        assert_response :success
-        assert_template 'terms'
+        get "/user/new", res
+        assert_redirected_to "/user/terms"
         post '/user/save',
           {:user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :openid_url => "http://localhost:1123/john.doe?openid.success=newuser", :pass_crypt => password, :pass_crypt_confirmation => password}}
         assert_response :redirect
@@ -181,11 +178,11 @@ class UserCreationTest < ActionController::IntegrationTest
     password = "testtest2"
     assert_difference('User.count',0) do
       assert_difference('ActionMailer::Base.deliveries.size',0) do
-        post "/user/terms",
+        post "/user/new",
           {:user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :openid_url => "http://localhost:1123/john.doe?openid.failure=newuser", :pass_crypt => "", :pass_crypt_confirmation => ""}}
         assert_response :redirect
         res = openid_request(@response.redirect_url)
-        post '/user/terms', res
+        get '/user/new', res
         assert_response :success
         assert_template 'user/new'
       end
@@ -202,13 +199,12 @@ class UserCreationTest < ActionController::IntegrationTest
     referer = "/traces/mine"
     assert_difference('User.count') do
       assert_difference('ActionMailer::Base.deliveries.size', 1) do
-       post "/user/terms",
+        post "/user/new",
           {:user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :openid_url => "http://localhost:1123/john.doe?openid.success=newuser", :pass_crypt => "", :pass_crypt_confirmation => ""}, :referer => referer }
-       assert_response :redirect
+        assert_response :redirect
         res = openid_request(@response.location)
-        post '/user/terms', res
-        assert_response :success
-        assert_template 'terms'
+        get "/user/new", res
+        assert_redirected_to "/user/terms"
         post_via_redirect "/user/save",
           {:user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :openid_url => "http://localhost:1123/john.doe?openid.success=newuser", :pass_crypt => "testtest", :pass_crypt_confirmation => "testtest"} }
       end