From ff93ee3cfaf4aae92e826c8e879e1bb648c0dddd Mon Sep 17 00:00:00 2001 From: Go MAEDA Date: Sun, 15 Oct 2023 01:42:07 +0000 Subject: [PATCH] API compatibility to legacy status and name query params (#39181, #37674) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit - adds 'name' filter that mimics the old behavior of matching on email, login, first- or lastname - maps the 'status' url parameter to the status_id filter, and the 'name' url parameter to the new name filter Patch by Jens Krämer. git-svn-id: https://svn.redmine.org/redmine/trunk@22343 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/users_controller.rb | 13 +++++++++ app/models/user_query.rb | 28 ++++++++++++++++++ config/locales/en.yml | 1 + test/integration/api_test/users_test.rb | 38 +++++++++++++++++++++++++ test/unit/user_query_test.rb | 23 +++++++++++++++ 5 files changed, 103 insertions(+) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 77ec1a601..f64c469da 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -45,6 +45,19 @@ class UsersController < ApplicationController use_session = !request.format.csv? retrieve_query(UserQuery, use_session) + # API backwards compatibility: handle legacy status and name filter parameters + unless request.format.html? + if status_id = params[:status].presence + @query.add_filter 'status', '=', [status_id] + end + if name = params[:name].presence + @query.add_filter 'name', '~', [name] + end + if group_id = params[:group_id].presence + @query.add_filter 'is_member_of_group', '=', [group_id] + end + end + if @query.valid? scope = @query.results_scope diff --git a/app/models/user_query.rb b/app/models/user_query.rb index 477c071e0..dbbb0eccb 100644 --- a/app/models/user_query.rb +++ b/app/models/user_query.rb @@ -51,6 +51,7 @@ class UserQuery < Query type: :list_optional, values: ->{ Redmine::Twofa.available_schemes.map {|s| [I18n.t("twofa__#{s}__name"), s] } } end + add_available_filter "name", type: :text, label: :field_name_or_email_or_login add_available_filter "login", type: :string add_available_filter "firstname", type: :string add_available_filter "lastname", type: :string @@ -165,4 +166,31 @@ class UserQuery < Query joins.any? ? joins.join(' ') : nil end + + def sql_for_name_field(field, operator, value) + case operator + when '*' + '1=1' + when '!*' + '1=0' + else + # match = (operator == '~') + match = !operator.start_with?('!') + matching_operator = operator.sub /^\!/, '' + name_sql = %w(login firstname lastname).map{|field| sql_for_field(:name, operator, value, User.table_name, field)} + + emails = EmailAddress.table_name + email_sql = <<-SQL + #{match ? "EXISTS" : "NOT EXISTS"} + (SELECT 1 FROM #{emails} WHERE + #{emails}.user_id = #{User.table_name}.id AND + #{sql_for_field(:name, matching_operator, value, emails, 'address')}) + SQL + + conditions = name_sql + [email_sql] + op = match ? " OR " : " AND " + "(#{conditions.map{|s| "(#{s})"}.join(op)})" + end + + end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 5a950a7ff..7674619ec 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1410,3 +1410,4 @@ en: twofa_text_group_disabled: "This setting is only effective when the global two factor authentication setting is set to 'optional'. Currently, two factor authentication is disabled." text_user_destroy_confirmation: "Are you sure you want to delete this user and remove all references to them? This cannot be undone. Often, locking a user instead of deleting them is the better solution. To confirm, please enter their login (%{login}) below." text_project_destroy_enter_identifier: "To confirm, please enter the project's identifier (%{identifier}) below." + field_name_or_email_or_login: Name, email or login diff --git a/test/integration/api_test/users_test.rb b/test/integration/api_test/users_test.rb index 81ac487ac..5b52f6d3b 100644 --- a/test/integration/api_test/users_test.rb +++ b/test/integration/api_test/users_test.rb @@ -82,6 +82,44 @@ class Redmine::ApiTest::UsersTest < Redmine::ApiTest::Base end end + test "GET /users.json with legacy filter params" do + get '/users.json', :headers => credentials('admin'), params: { status: 3 } + assert_response :success + json = ActiveSupport::JSON.decode(response.body) + assert json.key?('users') + users = User.where(status: 3) + assert_equal users.size, json['users'].size + + get '/users.json', :headers => credentials('admin'), params: { name: 'jsmith' } + assert_response :success + json = ActiveSupport::JSON.decode(response.body) + assert json.key?('users') + assert_equal 1, json['users'].size + assert_equal 2, json['users'][0]['id'] + + get '/users.json', :headers => credentials('admin'), params: { group_id: '10' } + assert_response :success + json = ActiveSupport::JSON.decode(response.body) + assert json.key?('users') + assert_equal 1, json['users'].size + assert_equal 8, json['users'][0]['id'] + + # there should be an implicit filter for status = 1 + User.where(id: [2, 8]).update_all status: 3 + + get '/users.json', :headers => credentials('admin'), params: { name: 'jsmith' } + assert_response :success + json = ActiveSupport::JSON.decode(response.body) + assert json.key?('users') + assert_equal 0, json['users'].size + + get '/users.json', :headers => credentials('admin'), params: { group_id: '10' } + assert_response :success + json = ActiveSupport::JSON.decode(response.body) + assert json.key?('users') + assert_equal 0, json['users'].size + end + test "GET /users/:id.xml should return the user" do Redmine::Configuration.with 'avatar_server_url' => 'https://gravatar.com' do with_settings :gravatar_enabled => '1', :gravatar_default => 'robohash' do diff --git a/test/unit/user_query_test.rb b/test/unit/user_query_test.rb index 9bb9c816d..e5d8dd312 100644 --- a/test/unit/user_query_test.rb +++ b/test/unit/user_query_test.rb @@ -108,6 +108,29 @@ class UserQueryTest < ActiveSupport::TestCase end end + def test_name_or_email_or_login_filter + [ + ['~', 'jsmith', [2]], + ['^', 'jsm', [2]], + ['$', 'ith', [2]], + ['~', 'john', [2]], + ['~', 'smith', [2]], + ['~', 'somenet', [1, 2, 3, 4]], + ['!~', 'somenet', [7, 8, 9]], + ['^', 'dlop', [3]], + ['$', 'bar', [7, 8, 9]], + ['=', 'bar', []], + ['=', 'someone@foo.bar', [7]], + ['*', '', [1, 2, 3, 4, 7, 8, 9]], + ['!*', '', []], + ].each do |op, string, result| + q = UserQuery.new name: '_' + q.add_filter('name', op, [string]) + users = find_users_with_query q + assert_equal result, users.map(&:id).sort, "#{op} #{string} should have found #{result}" + end + end + def test_group_filter q = UserQuery.new name: '_' q.add_filter('is_member_of_group', '=', ['10', '99']) -- 2.39.5