Redmine

Marius BALTEANU wrote in #note-1:

Users list was improved in Redmine 5.0 by using the same query mechanism as we have for issues, projects, etc (#37674) with filters and columns. In order to keep the compatibility to legacy status and name query params, the API was adapted in r22343, but as I see now, the behaviour for status= was not kept. If you change your query to status=1 it should return all active users. From my point of view, I prefer to not bring back the behaviour with status= because I find it not intuitive.

I will take a look on the 500 error, it should not happen.

Please let me know what do you think about the first part.

I agree that "status=" is quite ugly and counterintuitive, for me it can go, having the status=* as with the issues if fine (or any other way to get the full list)

Joan J wrote in #note-2:

I agree that "status=" is quite ugly and counterintuitive, for me it can go, having the status=* as with the issues if fine (or any other way to get the full list)

It seems that the only way to retrieve all users regardless their status using a single API request is to use the following query syntax users.json?f[]=status_id&op[status_id]==&v[status_id][]=1&v[status_id][]=2&v[status_id][]=3

I think we should add the "any" operator to status filter in order to enable the =* operator.

• I still need to investigate why short filters are not working for users listing
• If the the 500 error happens only on Postgresql because on MySQL is ok

Looks like with MySQL, the '*' in where status IN ('*') is cast to 0 so the query does not error out but gives just the anonymous user (which has status=0 in my local test db).

Interestingly enough, at Planio we are adding the user status filter as :list_optional in UserQuery, which allows querying for any status with ?f[]=status&op[status]=*. I'm not entirely sure why that did not make into the core patch. Probably we changed this at a later time:

diff --git a/app/models/user_query.rb b/app/models/user_query.rb
index bfb44cdad3..5051ceabe5 100644
--- a/app/models/user_query.rb
+++ b/app/models/user_query.rb
@@ -40,7 +40,7 @@ class UserQuery < Query

   def initialize_available_filters
     add_available_filter "status",
-      type: :list, values: ->{ user_statuses_values }
+      type: :list_optional, values: ->{ user_statuses_values }
     add_available_filter "auth_source_id",
       type: :list_optional, values: ->{ auth_sources_values }
     add_available_filter "is_member_of_group",

With that, a quick fix for the API to support the status=* short query parameter would be:

iff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 74e111de6..b806dca23 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -48,7 +48,8 @@ class UsersController < ApplicationController
     # API backwards compatibility: handle legacy filter parameters
     unless request.format.html?
       if status_id = params[:status].presence
-        @query.add_filter 'status', '=', [status_id]
+        if status_id == '*'
+          @query.add_filter 'status', '*'
+        else
+          @query.add_filter 'status', '=', [status_id]
+        end
       end
       if name = params[:name].presence
         @query.add_filter 'name', '~', [name]
diff --git a/test/integration/api_test/users_test.rb b/test/integration/api_test/users_test.rb
index 1a1682f32..3a38cfad1 100644
--- a/test/integration/api_test/users_test.rb
+++ b/test/integration/api_test/users_test.rb
@@ -90,6 +90,13 @@ class Redmine::ApiTest::UsersTest < Redmine::ApiTest::Base
     users = User.where(status: 3)
     assert_equal users.size, json['users'].size

+    get '/users.json', headers: credentials('admin'), params: { status: '*' }
+    assert_response :success
+    json = ActiveSupport::JSON.decode(response.body)
+    assert json.key?('users')
+    users = User.logged
+    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)

• the case when status is blank (status=), which should return all users as #32090 and rest_users point out.
• the case when status=*, which should return all users as it works for other entities that support the "any" operator.
• the case when status=1|3, which should return all users with status 1 or 3 as it works for other entities that support the "any" operator.

The last two cases should be handled internally by short filters syntax, but their are not working because of r22343.

In r22625, I've simplified the fix to make all 3 test cases to pass.

Also:
1. We should update the wiki page after we close this.
2. For Redmine 6.0, I don't think we should support the awkward status= syntax, I'm in favour to add a deprecation message in 5.1.2 together with these fixes and then remove it in 6.0.0.

What do you think? I'll merge this to 5.1-stable branch after your review.

This looks great, nice to let the non-blank status parameter values just pass through to be handled by add_short_filter. And yes regarding the deprecation of status=.

Jens Krämer wrote in #note-7:

This looks great, nice to let the non-blank status parameter values just pass through to be handled by add_short_filter. And yes regarding the deprecation of status=.

All done. The removal is part of #40124.