From: Tom Hughes Date: Wed, 20 Apr 2011 18:23:39 +0000 (+0100) Subject: Merge branch 'master' into openstreetbugs X-Git-Tag: live~5052^2~184^2~3 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/8980da4381923d8099a11a1334baf3c3e625a484?hp=a1cb0f04d476101116087286ff072d3a137355cb Merge branch 'master' into openstreetbugs --- diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index d66fbb28f..95cc88869 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -46,7 +46,7 @@ class ApiController < ApplicationController root = XML::Node.new 'gpx' root['version'] = '1.0' root['creator'] = 'OpenStreetMap.org' - root['xmlns'] = "http://www.topografix.com/GPX/1/0/" + root['xmlns'] = "http://www.topografix.com/GPX/1/0" doc.root = root diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 6c32b74b2..8062c9fe3 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -15,6 +15,16 @@ class ApplicationController < ActionController::Base session_expires_automatically redirect_to :controller => "user", :action => "suspended" + + # don't allow access to any auth-requiring part of the site unless + # the new CTs have been seen (and accept/decline chosen). + elsif !@user.terms_seen and flash[:showing_terms].nil? + flash[:notice] = t 'user.terms.you need to accept or decline' + if params[:referer] + redirect_to :controller => "user", :action => "terms", :referer => params[:referer] + else + redirect_to :controller => "user", :action => "terms", :referer => request.request_uri + end end elsif session[:token] @user = User.authenticate(:token => session[:token]) @@ -99,10 +109,21 @@ class ApplicationController < ActionController::Base end end - # check if the user has been banned - unless @user.nil? or @user.active_blocks.empty? - # NOTE: need slightly more helpful message than this. - render :text => t('application.setup_user_auth.blocked'), :status => :forbidden + # have we identified the user? + if @user + # check if the user has been banned + if not @user.active_blocks.empty? + # NOTE: need slightly more helpful message than this. + report_error t('application.setup_user_auth.blocked'), :forbidden + end + + # if the user hasn't seen the contributor terms then don't + # allow editing - they have to go to the web site and see + # (but can decline) the CTs to continue. + if REQUIRE_TERMS_SEEN and not @user.terms_seen + set_locale + report_error t('application.setup_user_auth.need_to_see_terms'), :forbidden + end end end @@ -134,8 +155,7 @@ class ApplicationController < ActionController::Base def check_api_readable if STATUS == :database_offline or STATUS == :api_offline - response.headers['Error'] = "Database offline for maintenance" - render :nothing => true, :status => :service_unavailable + report_error "Database offline for maintenance", :service_unavailable return false end end @@ -143,16 +163,14 @@ class ApplicationController < ActionController::Base def check_api_writable if STATUS == :database_offline or STATUS == :database_readonly or STATUS == :api_offline or STATUS == :api_readonly - response.headers['Error'] = "Database offline for maintenance" - render :nothing => true, :status => :service_unavailable + report_error "Database offline for maintenance", :service_unavailable return false end end def require_public_data unless @user.data_public? - response.headers['Error'] = "You must make your edits public to upload new data" - render :nothing => true, :status => :forbidden + report_error "You must make your edits public to upload new data", :forbidden return false end end @@ -165,7 +183,18 @@ class ApplicationController < ActionController::Base def report_error(message, status = :bad_request) # Todo: some sort of escaping of problem characters in the message response.headers['Error'] = message - render :text => message, :status => status + + if request.headers['X-Error-Format'] and + request.headers['X-Error-Format'].downcase == "xml" + result = OSM::API.new.get_xml_doc + result.root.name = "osmError" + result.root << (XML::Node.new("status") << interpret_status(status)) + result.root << (XML::Node.new("message") << message) + + render :text => result.to_s, :content_type => "text/xml" + else + render :text => message, :status => status + end end def set_locale @@ -181,6 +210,24 @@ class ApplicationController < ActionController::Base end end + if request.compatible_language_from(I18n.available_locales).nil? + request.user_preferred_languages = request.user_preferred_languages.collect do |pl| + pls = [ pl ] + + while pl.match(/^(.*)-[^-]+$/) + pls.push($1) if I18n.available_locales.include?($1.to_sym) + pl = $1 + end + + pls + end.flatten + + if @user and not request.compatible_language_from(I18n.available_locales).nil? + @user.languages = request.user_preferred_languages + @user.save + end + end + I18n.locale = request.compatible_language_from(I18n.available_locales) response.headers['Content-Language'] = I18n.locale.to_s diff --git a/app/controllers/browse_controller.rb b/app/controllers/browse_controller.rb index 2962c6836..f6bd89d4d 100644 --- a/app/controllers/browse_controller.rb +++ b/app/controllers/browse_controller.rb @@ -72,8 +72,10 @@ class BrowseController < ApplicationController @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 }] ) - @next_by_user = Changeset.find(:first, :order => "id ASC", :conditions => [ "id > :id AND user_id = :user_id", {:id => @changeset.id, :user_id => @changeset.user_id }] ) - @prev_by_user = Changeset.find(:first, :order => "id DESC", :conditions => [ "id < :id AND user_id = :user_id", {:id => @changeset.id, :user_id => @changeset.user_id }] ) + if @changeset.user.data_public? + @next_by_user = Changeset.find(:first, :order => "id ASC", :conditions => [ "id > :id AND user_id = :user_id", { :id => @changeset.id, :user_id => @changeset.user_id }] ) + @prev_by_user = Changeset.find(:first, :order => "id DESC", :conditions => [ "id < :id AND user_id = :user_id", { :id => @changeset.id, :user_id => @changeset.user_id }] ) + end rescue ActiveRecord::RecordNotFound render :action => "not_found", :status => :not_found end diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb index 29b9c787b..5a4dc36fb 100644 --- a/app/controllers/changeset_controller.rb +++ b/app/controllers/changeset_controller.rb @@ -436,7 +436,7 @@ private # query changesets which are closed # ('closed at' time has passed or changes limit is hit) def conditions_closed(closed) - return closed.nil? ? nil : ['closed_at < ? or num_changes > ?', + return closed.nil? ? nil : ['(closed_at < ? or num_changes > ?)', Time.now.getutc, Changeset::MAX_ELEMENTS] end diff --git a/app/controllers/geocoder_controller.rb b/app/controllers/geocoder_controller.rb index d07b74f11..29e7648af 100644 --- a/app/controllers/geocoder_controller.rb +++ b/app/controllers/geocoder_controller.rb @@ -234,7 +234,7 @@ class GeocoderController < ApplicationController end # ask nominatim - response = fetch_xml("http://nominatim.openstreetmap.org/search?format=xml&q=#{escape_query(query)}#{viewbox}#{exclude}&accept-language=#{request.user_preferred_languages.join(',')}") + response = fetch_xml("#{NOMINATIM_URL}search?format=xml&q=#{escape_query(query)}#{viewbox}#{exclude}&accept-language=#{request.user_preferred_languages.join(',')}") # create result array @results = Array.new @@ -355,7 +355,7 @@ class GeocoderController < ApplicationController @results = Array.new # ask OSM namefinder - response = fetch_xml("http://nominatim.openstreetmap.org/reverse?lat=#{lat}&lon=#{lon}&zoom=#{zoom}&accept-language=#{request.user_preferred_languages.join(',')}") + response = fetch_xml("#{NOMINATIM_URL}reverse?lat=#{lat}&lon=#{lon}&zoom=#{zoom}&accept-language=#{request.user_preferred_languages.join(',')}") # parse the response response.elements.each("reversegeocode/result") do |result| diff --git a/app/controllers/oauth_controller.rb b/app/controllers/oauth_controller.rb index f70a644cd..5c84be0cf 100644 --- a/app/controllers/oauth_controller.rb +++ b/app/controllers/oauth_controller.rb @@ -1,5 +1,5 @@ class OauthController < ApplicationController - layout 'site' + layout 'slim' before_filter :authorize_web, :only => [:oauthorize, :revoke] before_filter :set_locale, :only => [:oauthorize, :revoke] diff --git a/app/controllers/site_controller.rb b/app/controllers/site_controller.rb index 1478c5773..73f38f78c 100644 --- a/app/controllers/site_controller.rb +++ b/app/controllers/site_controller.rb @@ -30,4 +30,41 @@ class SiteController < ApplicationController def key expires_in 7.days, :public => true end + + def edit + editor = params[:editor] || @user.preferred_editor || DEFAULT_EDITOR + + if editor == "remote" + render :action => :index + else + # Decide on a lat lon to initialise potlatch with. Various ways of doing this + if params['lon'] and params['lat'] + @lon = params['lon'].to_f + @lat = params['lat'].to_f + @zoom = params['zoom'].to_i + + elsif params['mlon'] and params['mlat'] + @lon = params['mlon'].to_f + @lat = params['mlat'].to_f + @zoom = params['zoom'].to_i + + elsif params['gpx'] + @lon = Trace.find(params['gpx']).longitude + @lat = Trace.find(params['gpx']).latitude + + elsif cookies.key?("_osm_location") + @lon, @lat, @zoom, layers = cookies["_osm_location"].split("|") + + elsif @user and !@user.home_lon.nil? and !@user.home_lat.nil? + @lon = @user.home_lon + @lat = @user.home_lat + + else + #catch all. Do nothing. lat=nil, lon=nil + #Currently this results in potlatch starting up at 0,0 (Atlantic ocean). + end + + @zoom = '14' if @zoom.nil? + end + end end diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index c8603afec..de43fee14 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -1,6 +1,7 @@ class UserController < ApplicationController - layout 'site', :except => :api_details + layout :choose_layout + before_filter :disable_terms_redirect, :only => [:terms, :save, :logout] before_filter :authorize, :only => [:api_details, :api_gpx_files] before_filter :authorize_web, :except => [:api_details, :api_gpx_files] before_filter :set_locale, :except => [:api_details, :api_gpx_files] @@ -24,7 +25,7 @@ class UserController < ApplicationController if request.xhr? render :update do |page| - page.replace_html "contributorTerms", :partial => "terms", :locals => { :has_decline => params[:has_decline] } + page.replace_html "contributorTerms", :partial => "terms" end else @title = t 'user.terms.title' @@ -53,17 +54,36 @@ class UserController < ApplicationController if Acl.find_by_address(request.remote_ip, :conditions => {:k => "no_account_creation"}) render :action => 'new' elsif params[:decline] - redirect_to t('user.terms.declined') + if @user + @user.terms_seen = true + + if @user.save + flash[:notice] = t 'user.new.terms declined', :url => t('user.new.terms declined url') + end + + if params[:referer] + redirect_to params[:referer] + else + redirect_to :action => :account, :display_name => @user.display_name + end + else + redirect_to t('user.terms.declined') + end elsif @user if !@user.terms_agreed? @user.consider_pd = params[:user][:consider_pd] @user.terms_agreed = Time.now.getutc + @user.terms_seen = true if @user.save flash[:notice] = t 'user.new.terms accepted' end end - redirect_to :action => :account, :display_name => @user.display_name + if params[:referer] + redirect_to params[:referer] + else + redirect_to :action => :account, :display_name => @user.display_name + end else @user = User.new(params[:user]) @@ -73,13 +93,15 @@ class UserController < ApplicationController @user.creation_ip = request.remote_ip @user.languages = request.user_preferred_languages @user.terms_agreed = Time.now.getutc - + @user.terms_seen = true + if @user.save flash[:notice] = t 'user.new.flash create success message', :email => @user.email Notifier.deliver_signup_confirm(@user, @user.tokens.create(:referer => params[:referer])) - redirect_to :action => 'login' + session[:token] = @user.tokens.create.token + redirect_to :action => 'login', :referer => params[:referer] else - render :action => 'new' + render :action => 'new', :referer => params[:referer] end end end @@ -108,6 +130,12 @@ class UserController < ApplicationController @user.home_lat = params[:user][:home_lat] @user.home_lon = params[:user][:home_lon] + if params[:user][:preferred_editor] == "default" + @user.preferred_editor = nil + else + @user.preferred_editor = params[:user][:preferred_editor] + end + if @user.save set_locale @@ -207,15 +235,20 @@ class UserController < ApplicationController session[:user] = user.id session_expires_after 1.month if params[:remember_me] - # The user is logged in, if the referer param exists, redirect - # them to that unless they've also got a block on them, in - # which case redirect them to the block so they can clear it. - if user.blocked_on_view - redirect_to user.blocked_on_view, :referer => params[:referer] - elsif params[:referer] - redirect_to params[:referer] + target = params[:referer] || url_for(:controller => :site, :action => :index) + + # The user is logged in, so decide where to send them: + # + # - If they haven't seen the contributor terms, send them there. + # - If they have a block on them, show them that. + # - If they were referred to the login, send them back there. + # - Otherwise, send them to the home page. + if REQUIRE_TERMS_SEEN and not user.terms_seen + redirect_to :controller => :user, :action => :terms, :referer => target + elsif user.blocked_on_view + redirect_to user.blocked_on_view, :referer => target else - redirect_to :controller => 'site', :action => 'index' + redirect_to target end elsif user = User.authenticate(:username => email_or_display_name, :password => pass, :pending => true) flash.now[:error] = t 'user.login.account not active', :reconfirm => url_for(:action => 'confirm_resend', :display_name => user.display_name) @@ -264,14 +297,29 @@ class UserController < ApplicationController user.save! referer = token.referer token.destroy - session[:user] = user.id - unless referer.nil? + if session[:token] + token = UserToken.find_by_token(session[:token]) + session.delete(:token) + else + token = nil + end + + if token.nil? or token.user != user flash[:notice] = t('user.confirm.success') - redirect_to referer + redirect_to :action => :login, :referer => referer else - flash[:notice] = t('user.confirm.success') + "

" + t('user.confirm.before you start') - redirect_to :action => 'account', :display_name => user.display_name + token.destroy + + session[:user] = user.id + + if referer.nil? + flash[:notice] = t('user.confirm.success') + "

" + t('user.confirm.before you start') + redirect_to :action => :account, :display_name => user.display_name + else + flash[:notice] = t('user.confirm.success') + redirect_to referer + end end end else @@ -452,4 +500,28 @@ private rescue ActiveRecord::RecordNotFound redirect_to :controller => 'user', :action => 'view', :display_name => params[:display_name] unless @this_user end + + ## + # Choose the layout to use. See + # https://rails.lighthouseapp.com/projects/8994/tickets/5371-layout-with-onlyexcept-options-makes-other-actions-render-without-layouts + def choose_layout + oauth_url = url_for(:controller => :oauth, :action => :oauthorize, :only_path => true) + + if [ 'api_details' ].include? action_name + nil + elsif params[:referer] and URI.parse(params[:referer]).path == oauth_url + 'slim' + else + 'site' + end + end + + ## + # + def disable_terms_redirect + # this is necessary otherwise going to the user terms page, when + # having not agreed already would cause an infinite redirect loop. + # it's .now so that this doesn't propagate to other pages. + flash.now[:showing_terms] = true + end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 2ccfcca2b..c24e7ff5b 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -119,6 +119,16 @@ module ApplicationHelper end end + def preferred_editor + if params[:editor] + params[:editor] + elsif @user and @user.preferred_editor + @user.preferred_editor + else + DEFAULT_EDITOR + end + end + private def javascript_strings_for_key(key) diff --git a/app/models/client_application.rb b/app/models/client_application.rb index 9474a0137..09eec40d3 100644 --- a/app/models/client_application.rb +++ b/app/models/client_application.rb @@ -2,6 +2,7 @@ require 'oauth' class ClientApplication < ActiveRecord::Base belongs_to :user has_many :tokens, :class_name => "OauthToken" + has_many :access_tokens validates_presence_of :name, :url, :key, :secret validates_uniqueness_of :key before_validation_on_create :generate_keys @@ -53,6 +54,20 @@ class ClientApplication < ActiveRecord::Base RequestToken.create :client_application => self, :callback_url => self.token_callback_url end + def access_token_for_user(user) + unless token = access_tokens.find(:first, :conditions => { :user_id => user.id, :invalidated_at => nil }) + params = { :user => user } + + permissions.each do |p| + params[p] = true + end + + token = access_tokens.create(params) + end + + token + end + # the permissions that this client would like from the user def permissions ClientApplication.all_permissions.select { |p| self[p] } diff --git a/app/models/user.rb b/app/models/user.rb index 1a50f7053..d2535bbd4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -10,7 +10,7 @@ class User < ActiveRecord::Base has_many :friends, :include => :befriendee, :conditions => "users.status IN ('active', 'confirmed')" has_many :tokens, :class_name => "UserToken" has_many :preferences, :class_name => "UserPreference" - has_many :changesets + has_many :changesets, :order => 'created_at DESC' has_many :client_applications has_many :oauth_tokens, :class_name => "OauthToken", :order => "authorized_at desc", :include => [:client_application] @@ -33,6 +33,7 @@ class User < ActiveRecord::Base validates_numericality_of :home_lat, :allow_nil => true validates_numericality_of :home_lon, :allow_nil => true validates_numericality_of :home_zoom, :only_integer => true, :allow_nil => true + validates_inclusion_of :preferred_editor, :in => Editors::ALL_EDITORS, :allow_nil => true before_save :encrypt_password @@ -106,7 +107,7 @@ class User < ActiveRecord::Base (languages & array.collect { |i| i.to_s }).first end - def nearby(radius = 50, num = 10) + def nearby(radius = NEARBY_RADIUS, num = NEARBY_USERS) if self.home_lon and self.home_lat gc = OSM::GreatCircle.new(self.home_lat, self.home_lon) bounds = gc.bounds(radius) @@ -202,4 +203,10 @@ class User < ActiveRecord::Base return score.to_i end + + ## + # return an oauth access token for a specified application + def access_token(application_key) + return ClientApplication.find_by_key(application_key).access_token_for_user(self) + end end diff --git a/app/views/browse/_changeset_details.html.erb b/app/views/browse/_changeset_details.html.erb index 6e3cbdb2e..9fb9cb860 100644 --- a/app/views/browse/_changeset_details.html.erb +++ b/app/views/browse/_changeset_details.html.erb @@ -84,7 +84,7 @@ <% @relations.each do |relation| %> - + <% end %>
<%= link_to h(printable_name(relation, true)), { :action => "relation", :id => relation.id.to_s }, :class => "relation " %>
<%= link_to h(printable_name(relation, true)), { :action => "relation", :id => relation.id.to_s }, :class => link_class('relation', relation), :title => link_title(relation) %>
diff --git a/app/views/browse/_map.html.erb b/app/views/browse/_map.html.erb index 0b7f76f23..2e20f07f3 100644 --- a/app/views/browse/_map.html.erb +++ b/app/views/browse/_map.html.erb @@ -2,7 +2,7 @@ <%= javascript_include_tag '/openlayers/OpenStreetMap.js' %> <%= javascript_include_tag 'map.js' %>
- <% if map.instance_of? Changeset or map.visible %> + <% if map.instance_of? Changeset or (map.instance_of? Node and map.version > 1) or map.visible %>
<%= t 'browse.map.loading' %> @@ -15,7 +15,7 @@ <%= t 'browse.map.deleted' %> <% end %>
-<% if map.instance_of? Changeset or map.visible %> +<% if map.instance_of? Changeset or (map.instance_of? Node and map.version > 1) or map.visible %> +