Redmine

Jens Krämer wrote:

#37674 changed the /users endpoint behavior by dropping support for the previously supported status and name URL parameters. This might break API clients that relied on these parameters. Further the functionality of the 'Name' filter, which allowed to do a combined search across users' name, email and login was lost for both API and Browser clients.

Tentatively confirmed (by code review only).

Besides the previously supported status and name URL parameters, Redmine also supported a group_id URL parameter (per #7893), which allowed clients to "get only users who are members of the given group¹".
AFAICT this functionality got lost with the implementation of #37674 too.

Extending the attached patch like this (untested code):

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 77ec1a601..e1d3a7521 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -45,6 +45,20 @@ class UsersController < ApplicationController
     use_session = !request.format.csv?
     retrieve_query(UserQuery, use_session)

+    # API backwards compatibility: handle legacy status, name and
+    # group_id 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

would probably suffice to fix this additional API compatibility regression.
It should additionally map the 'group_id' URL parameter to a query on 'is_member_of_group' if present.

I think that it would also be good to have some test coverage for these API features in case this patch (with or without my extension) is accepted.

¹ from Rest_Users#GET

thanks for looking into this, and yes, adding group_id as well seems right to me. I attached a revised patch including this as well as an integration test covering this.

Committed the fix as a part of #37674. Thank you.

Go MAEDA wrote in #note-5:

Committed the fix as a part of #37674. Thank you.

Go MAEDA: for sake of correctness, can you also adapt the comment to actually reflect the current situation like something I proposed in #note-3?

Fixed the comment in r22354, as Mischa suggested in #note-8. Thank you.