From b4106383d99ccbf152d79b0f2c9deca95df9fb61 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Mon, 9 Jul 2018 22:19:10 +0100 Subject: [PATCH] Add /api/0.6/users to fetch multiple users Fixes #1921 --- .rubocop_todo.yml | 2 +- app/controllers/user_controller.rb | 24 +++++++++--- app/views/user/_api_user.builder | 44 ++++++++++++++++++++++ app/views/user/api_read.builder | 47 +----------------------- app/views/user/api_users.builder | 4 ++ config/routes.rb | 1 + test/controllers/user_controller_test.rb | 46 +++++++++++++++++++++++ 7 files changed, 116 insertions(+), 52 deletions(-) create mode 100644 app/views/user/_api_user.builder create mode 100644 app/views/user/api_users.builder diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 576f6080a..a64be7273 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -54,7 +54,7 @@ Metrics/AbcSize: # Offense count: 41 # Configuration parameters: CountComments, ExcludedMethods. Metrics/BlockLength: - Max: 257 + Max: 258 # Offense count: 11 # Configuration parameters: CountBlocks. diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 63fad8c83..7e22c63b4 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -1,21 +1,21 @@ class UserController < ApplicationController layout "site", :except => [:api_details] - skip_before_action :verify_authenticity_token, :only => [:api_read, :api_details, :api_gpx_files, :auth_success] + skip_before_action :verify_authenticity_token, :only => [:api_read, :api_users, :api_details, :api_gpx_files, :auth_success] before_action :disable_terms_redirect, :only => [:terms, :save, :logout, :api_details] before_action :authorize, :only => [:api_details, :api_gpx_files] - before_action :authorize_web, :except => [:api_read, :api_details, :api_gpx_files] - before_action :set_locale, :except => [:api_read, :api_details, :api_gpx_files] + before_action :authorize_web, :except => [:api_read, :api_users, :api_details, :api_gpx_files] + before_action :set_locale, :except => [:api_read, :api_users, :api_details, :api_gpx_files] before_action :require_user, :only => [:account, :go_public, :make_friend, :remove_friend] before_action :require_self, :only => [:account] - before_action :check_database_readable, :except => [:login, :api_read, :api_details, :api_gpx_files] + before_action :check_database_readable, :except => [:login, :api_read, :api_users, :api_details, :api_gpx_files] before_action :check_database_writable, :only => [:new, :account, :confirm, :confirm_email, :lost_password, :reset_password, :go_public, :make_friend, :remove_friend] - before_action :check_api_readable, :only => [:api_read, :api_details, :api_gpx_files] + before_action :check_api_readable, :only => [:api_read, :api_users, :api_details, :api_gpx_files] before_action :require_allow_read_prefs, :only => [:api_details] before_action :require_allow_read_gpx, :only => [:api_gpx_files] before_action :require_cookies, :only => [:new, :login, :confirm] before_action :require_administrator, :only => [:set_status, :delete, :list] - around_action :api_call_handle_error, :only => [:api_read, :api_details, :api_gpx_files] + around_action :api_call_handle_error, :only => [:api_read, :api_users, :api_details, :api_gpx_files] before_action :lookup_user_by_id, :only => [:api_read] before_action :lookup_user_by_name, :only => [:set_status, :delete] before_action :allow_thirdparty_images, :only => [:view, :account] @@ -389,6 +389,18 @@ class UserController < ApplicationController render :action => :api_read, :content_type => "text/xml" end + def api_users + raise OSM::APIBadUserInput, "The parameter users is required, and must be of the form users=id[,id[,id...]]" unless params["users"] + + ids = params["users"].split(",").collect(&:to_i) + + raise OSM::APIBadUserInput, "No users were given to search for" if ids.empty? + + @users = User.visible.find(ids) + + render :action => :api_users, :content_type => "text/xml" + end + def api_gpx_files doc = OSM::API.new.get_xml_doc current_user.traces.reload.each do |trace| diff --git a/app/views/user/_api_user.builder b/app/views/user/_api_user.builder new file mode 100644 index 000000000..91cd562e9 --- /dev/null +++ b/app/views/user/_api_user.builder @@ -0,0 +1,44 @@ +xml.tag! "user", :id => api_user.id, + :display_name => api_user.display_name, + :account_created => api_user.creation_time.xmlschema do + xml.tag! "description", api_user.description if api_user.description + if current_user && current_user == api_user + xml.tag! "contributor-terms", :agreed => api_user.terms_agreed.present?, + :pd => api_user.consider_pd + else + xml.tag! "contributor-terms", :agreed => api_user.terms_agreed.present? + end + xml.tag! "img", :href => user_image_url(api_user) if api_user.image.file? || api_user.image_use_gravatar + xml.tag! "roles" do + api_user.roles.each do |role| + xml.tag! role.role + end + end + xml.tag! "changesets", :count => api_user.changesets.size + xml.tag! "traces", :count => api_user.traces.size + xml.tag! "blocks" do + xml.tag! "received", :count => api_user.blocks.size, + :active => api_user.blocks.active.size + if api_user.moderator? + xml.tag! "issued", :count => api_user.blocks_created.size, + :active => api_user.blocks_created.active.size + end + end + if current_user && current_user == api_user + if api_user.home_lat && api_user.home_lon + xml.tag! "home", :lat => api_user.home_lat, + :lon => api_user.home_lon, + :zoom => api_user.home_zoom + end + if api_user.languages + xml.tag! "languages" do + api_user.languages.split(",") { |lang| xml.tag! "lang", lang } + end + end + xml.tag! "messages" do + xml.tag! "received", :count => api_user.messages.size, + :unread => api_user.new_messages.size + xml.tag! "sent", :count => api_user.sent_messages.size + end + end +end diff --git a/app/views/user/api_read.builder b/app/views/user/api_read.builder index e9cebd5ae..1598f3c53 100644 --- a/app/views/user/api_read.builder +++ b/app/views/user/api_read.builder @@ -1,47 +1,4 @@ xml.instruct! :xml, :version => "1.0" -xml.osm("version" => API_VERSION, "generator" => GENERATOR) do - xml.tag! "user", :id => @user.id, - :display_name => @user.display_name, - :account_created => @user.creation_time.xmlschema do - xml.tag! "description", @user.description if @user.description - if current_user && current_user == @user - xml.tag! "contributor-terms", :agreed => @user.terms_agreed.present?, - :pd => @user.consider_pd - else - xml.tag! "contributor-terms", :agreed => @user.terms_agreed.present? - end - xml.tag! "img", :href => user_image_url(@user) if @user.image.file? || @user.image_use_gravatar - xml.tag! "roles" do - @user.roles.each do |role| - xml.tag! role.role - end - end - xml.tag! "changesets", :count => @user.changesets.size - xml.tag! "traces", :count => @user.traces.size - xml.tag! "blocks" do - xml.tag! "received", :count => @user.blocks.size, - :active => @user.blocks.active.size - if @user.moderator? - xml.tag! "issued", :count => @user.blocks_created.size, - :active => @user.blocks_created.active.size - end - end - if current_user && current_user == @user - if @user.home_lat && @user.home_lon - xml.tag! "home", :lat => @user.home_lat, - :lon => @user.home_lon, - :zoom => @user.home_zoom - end - if @user.languages - xml.tag! "languages" do - @user.languages.split(",") { |lang| xml.tag! "lang", lang } - end - end - xml.tag! "messages" do - xml.tag! "received", :count => @user.messages.size, - :unread => @user.new_messages.size - xml.tag! "sent", :count => @user.sent_messages.size - end - end - end +xml.osm("version" => API_VERSION, "generator" => GENERATOR) do |osm| + osm << render(:partial => "api_user", :object => @user) end diff --git a/app/views/user/api_users.builder b/app/views/user/api_users.builder new file mode 100644 index 000000000..1d2475743 --- /dev/null +++ b/app/views/user/api_users.builder @@ -0,0 +1,4 @@ +xml.instruct! :xml, :version => "1.0" +xml.osm("version" => API_VERSION, "generator" => GENERATOR) do |osm| + osm << render(:partial => "api_user", :collection => @users) +end diff --git a/config/routes.rb b/config/routes.rb index 16ba85754..0522115f1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -67,6 +67,7 @@ OpenStreetMap::Application.routes.draw do get "user/:id" => "user#api_read", :id => /\d+/ get "user/details" => "user#api_details" get "user/gpx_files" => "user#api_gpx_files" + get "users" => "user#api_users", :as => :api_users get "user/preferences" => "user_preferences#read" get "user/preferences/:preference_key" => "user_preferences#read_one" diff --git a/test/controllers/user_controller_test.rb b/test/controllers/user_controller_test.rb index e8d452fc7..d3cf5ef13 100644 --- a/test/controllers/user_controller_test.rb +++ b/test/controllers/user_controller_test.rb @@ -20,6 +20,10 @@ class UserControllerTest < ActionController::TestCase { :path => "/api/0.6/user/gpx_files", :method => :get }, { :controller => "user", :action => "api_gpx_files" } ) + assert_routing( + { :path => "/api/0.6/users", :method => :get }, + { :controller => "user", :action => "api_users" } + ) assert_routing( { :path => "/login", :method => :get }, @@ -1146,6 +1150,48 @@ class UserControllerTest < ActionController::TestCase end end + def test_api_users + user1 = create(:user, :description => "test1", :terms_agreed => Date.yesterday) + user2 = create(:user, :description => "test2", :terms_agreed => Date.yesterday) + user3 = create(:user, :description => "test3", :terms_agreed => Date.yesterday) + + get :api_users, :params => { :users => user1.id } + assert_response :success + assert_equal "text/xml", response.content_type + assert_select "user", :count => 1 do + assert_select "user[id='#{user1.id}']", :count => 1 + assert_select "user[id='#{user2.id}']", :count => 0 + assert_select "user[id='#{user3.id}']", :count => 0 + end + + get :api_users, :params => { :users => user2.id } + assert_response :success + assert_equal "text/xml", response.content_type + assert_select "user", :count => 1 do + assert_select "user[id='#{user1.id}']", :count => 0 + assert_select "user[id='#{user2.id}']", :count => 1 + assert_select "user[id='#{user3.id}']", :count => 0 + end + + get :api_users, :params => { :users => "#{user1.id},#{user3.id}" } + assert_response :success + assert_equal "text/xml", response.content_type + assert_select "user", :count => 2 do + assert_select "user[id='#{user1.id}']", :count => 1 + assert_select "user[id='#{user2.id}']", :count => 0 + assert_select "user[id='#{user3.id}']", :count => 1 + end + + get :api_users, :params => { :users => create(:user, :suspended).id } + assert_response :not_found + + get :api_users, :params => { :users => create(:user, :deleted).id } + assert_response :not_found + + get :api_users, :params => { :users => 0 } + assert_response :not_found + end + def test_api_gpx_files user = create(:user) trace1 = create(:trace, :user => user) do |trace| -- 2.39.5