From 4fb01d1cc55980ec963230f0806f60bb643d02b9 Mon Sep 17 00:00:00 2001
From: Jens Kraemer <jk@jkraemer.net>
Date: Wed, 11 Oct 2023 17:00:56 +0800
Subject: [PATCH] API compatibility to legacy status and name query params

- 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
---
 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.2

