Feature #37674

Upgrade Admin/Users list to use the query system

Added by Jens Krämer 15 days ago. Updated about 12 hours ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Go MAEDA% Done:

0%

Category:Administration
Target version:5.1.0
Resolution:Fixed

Description

This patch series that was extracted from Planio introduces a UserQuery
and upgrades the Admin/Users list with corresponding filter, sort and (CSV) export capabilities.
Also introduced are a context menu and an option to delete multiple users at once. The third patch just
adds flash messages to the (single) user deletion operation, for symmetry with the messages rendered by the bulk delete.

This relates to issues #4295, #18193, #21055 (probably there are more I didn't find).

0002-user-bulk-destroy.patch Magnifier (6.5 KB) Jens Krämer, 2022-09-14 04:37

0001-introduces-a-UserQuery-model-for-admin-users.patch Magnifier (30.4 KB) Jens Krämer, 2022-09-14 04:37

0003-confirm-user-update-deletion.patch Magnifier (971 Bytes) Jens Krämer, 2022-09-14 04:37

0004-adds-column-and-filter-for-authentication-mode-to-ad.patch Magnifier (3.2 KB) Jens Krämer, 2022-09-14 04:37

screenshot-20220915.png (238 KB) Go MAEDA, 2022-09-15 05:46

0001-introduces-a-UserQuery-model-for-admin-users.patch Magnifier - patch with fix for SQLite (30.6 KB) Jens Krämer, 2022-09-15 07:39

0002-user-bulk-destroy.patch Magnifier (6.5 KB) Jens Krämer, 2022-09-16 01:39

0003-confirm-user-update-deletion.patch Magnifier (971 Bytes) Jens Krämer, 2022-09-16 01:39

0001-introduces-a-UserQuery-model-for-admin-users.patch Magnifier (31.7 KB) Jens Krämer, 2022-09-16 01:39

0001-adds-tests-for-UserQuery-model-37674.patch Magnifier (5.86 KB) Jens Krämer, 2022-09-21 06:06

0001-adds-missing-join-when-ordering-by-authsource-37674.patch Magnifier (1.67 KB) Jens Krämer, 2022-09-27 03:42


Related issues

Related to Redmine - Feature #4295: Support more filters on the user list page Closed 2009-11-26
Related to Redmine - Feature #18193: Add ability to filter users list by Authentication mode Closed
Related to Redmine - Patch #21055: Add Authentication mode filter in the Users admin page Closed
Related to Redmine - Feature #4023: User Custom Fields in List view Closed 2009-10-13
Related to Redmine - Feature #11501: More filters on page admin -> users Closed
Related to Redmine - Feature #31043: Search/filter users in backend Closed

Associated revisions

Revision 21823
Added by Go MAEDA 9 days ago

Introduces a UserQuery model for admin/users (#37674).

Patch by Jens Krämer.

Revision 21824
Added by Go MAEDA 9 days ago

User bulk destroy (#37674).

includes a confirmation page that also gives the opportunity to lock
users instead of deleting them.

Patch by Jens Krämer.

Revision 21825
Added by Go MAEDA 9 days ago

Confirm user update / deletion (#37674).

Patch by Jens Krämer.

Revision 21826
Added by Go MAEDA 9 days ago

Update locales (#37674).

Revision 21828
Added by Go MAEDA 8 days ago

Adds tests for UserQuery model (#37674).

Patch by Jens Krämer.

Revision 21835
Added by Go MAEDA 5 days ago

Fix random test failure (#37674).

Revision 21856
Added by Go MAEDA 1 day ago

Adds missing join when ordering by authsource (#37674).

Patch by Jens Krämer.

Revision 21860
Added by Marius BALTEANU about 24 hours ago

Fix failing test on PostgreSQL by ordering by only non null values (#37674).

Revision 21861
Added by Marius BALTEANU about 23 hours ago

Fix undefined method when auth source name column is added as column (#37674).

History

#1 Updated by Go MAEDA 14 days ago

Screenshot of this feature:

#2 Updated by Go MAEDA 14 days ago

Thank you for posting the nice improvement. However, the following error occurred with SQLite.

Error:
UsersControllerTest#test_index_with_name_filter:
ActiveRecord::StatementInvalid: SQLite3::SQLException: no such function: CONCAT
    app/controllers/users_controller.rb:54:in `index'
    lib/redmine/sudo_mode.rb:61:in `sudo_mode'
    test/functional/users_controller_test.rb:54:in `test_index_with_name_filter'

rails test test/functional/users_controller_test.rb:53

Could you update the patch to support SQLite? In my environment, I was able to fix the error with the following code.

--- app/models/user_query.rb.org    2022-09-15 13:34:22.000000000 +0900
+++ app/models/user_query.rb    2022-09-15 13:34:41.000000000 +0900
@@ -134,8 +134,13 @@

   def sql_for_name_field(field, operator, value)
     match = operator == '~'
-    name_sql = sql_contains("CONCAT(login, ' ', firstname, ' ', lastname)",
-                            value.first, match: match)
+    if Redmine::Database.sqlite?
+      name_sql = sql_contains("login || ' ' || firstname || ' ' || lastname",
+                              value.first, match: match)
+    else
+      name_sql = sql_contains("CONCAT(login, ' ', firstname, ' ', lastname)",
+                              value.first, match: match)
+    end

     emails = EmailAddress.table_name
     email_sql = <<-SQL

#3 Updated by Jens Krämer 14 days ago

Thank you for looking into this. Attached is the first patch with your solution built in.

#4 Updated by Go MAEDA 13 days ago

This patch uses the "name" filter to search three columns: first name, last name, and email. I think this follows the current search feature. However, I think it may be confusing for users to use the "name" filter to search for email addresses. Actually, neither I nor two of my colleagues could not find a way to search by email until I read the patch.

What about extracting the email address search from the "name" filter and adding a separate "email" filter?

#5 Updated by Jens Krämer 13 days ago

Good point. I indeed tried to stay close to the existing functionality with this combined name filter, but maybe that's not really necessary after all.

I also just noticed that support for the other filter operators like 'starts with' / 'ends with' is still missing. I'll come up with an updated patch soon.

#6 Updated by Marius BALTEANU 13 days ago

I agree that each user attribute should have its specific filter.

#7 Updated by Jens Krämer 13 days ago

Please find attached a new series of patches with the following changes:

  • reduced number of commits by merging the last commit (auth source filter and column) into the first.
  • separate filters for login, firstname, lastname, mail
  • fixed the mail filter to support all string operators

#8 Updated by Go MAEDA 12 days ago

  • Target version set to 5.1.0

Setting the target version to 5.1.0.

#9 Updated by Go MAEDA 9 days ago

  • Status changed from New to Closed
  • Assignee set to Go MAEDA

Committed patches with fixes of RuboCop warnings. Thank you for providing this improvement.

#10 Updated by Go MAEDA 9 days ago

  • Related to Feature #4295: Support more filters on the user list page added

#11 Updated by Go MAEDA 9 days ago

  • Related to Feature #18193: Add ability to filter users list by Authentication mode added

#12 Updated by Go MAEDA 9 days ago

  • Related to Patch #21055: Add Authentication mode filter in the Users admin page added

#13 Updated by Go MAEDA 9 days ago

  • Tracker changed from Patch to Feature
  • Resolution set to Fixed

#14 Updated by Mischa The Evil 9 days ago

  • Related to Feature #4023: User Custom Fields in List view added

#15 Updated by Mischa The Evil 9 days ago

  • Related to Feature #11501: More filters on page admin -> users added

#16 Updated by Mischa The Evil 9 days ago

#17 Updated by Jens Krämer 8 days ago

I just noticed that I forgot to add the tests for the new UserQuery model.

#18 Updated by Go MAEDA 8 days ago

  • Status changed from Reopened to Closed

Jens Krämer wrote:

I just noticed that I forgot to add the tests for the new UserQuery model.

Committed the code in r21828. Thank you.

#19 Updated by Marius BALTEANU 2 days ago

  • Status changed from Closed to Reopened

There is an SQL error when sorting the users list by column Authentication Mode. Te reproduce, go to users page, add the Authentication Mode to the users list and order by the same column:

PG::UndefinedTable: ERROR: missing FROM-clause entry for table "auth_sources" LINE 1: ...ers"."type" = $2 AND (users.status <> 0) ORDER BY auth_sourc... ^

I've tested only on postgresql, but from a quick check, it seems that the table auth_sources is not joined, but the table is used in the sort. Can you check, please?

#20 Updated by Jens Krämer 1 day ago

My bad, I didn't override joins_for_order_statement to account for that. Attached patch fixes this.

#21 Updated by Go MAEDA 1 day ago

  • Status changed from Reopened to Closed

Jens Krämer wrote:

My bad, I didn't override joins_for_order_statement to account for that. Attached patch fixes this.

Committed the fix. Thank you for quickly writing the fix.

#22 Updated by Go MAEDA 1 day ago

  • Status changed from Closed to Reopened

The test fails with PostgreSQL.

Failure:
UserQueryTest#test_auth_source_ordering [/var/lib/jenkins/workspace/trunk/DATABASE_ADAPTER/postgresql/RUBY_VER/ruby-3.1/test/unit/user_query_test.rb:182]:
--- expected
+++ actual
@@ -1 +1 @@
-#<User id: 1, login: "admin", hashed_password: [FILTERED], firstname: "Redmine", lastname: "Admin", admin: true, status: 1, last_login_on: "2006-07-19 20:57:52.000000000 +0000", language: "en", auth_source_id: 1, created_on: "2006-07-19 17:12:21.000000000 +0000", updated_on: "2006-07-19 20:57:52.000000000 +0000", type: "User", mail_notification: "all", salt: "82090c953c4a0000a7db253b0691a6b4", must_change_passwd: false, passwd_changed_on: nil, twofa_scheme: nil, twofa_totp_key: nil, twofa_totp_last_used_at: nil, twofa_required: false>
+#<User id: 2, login: "jsmith", hashed_password: [FILTERED], firstname: "John", lastname: "Smith", admin: false, status: 1, last_login_on: "2006-07-19 20:42:15.000000000 +0000", language: "en", auth_source_id: nil, created_on: "2006-07-19 17:32:09.000000000 +0000", updated_on: "2006-07-19 20:42:15.000000000 +0000", type: "User", mail_notification: "all", salt: "67eb4732624d5a7753dcea7ce0bb7d7d", must_change_passwd: false, passwd_changed_on: nil, twofa_scheme: nil, twofa_totp_key: nil, twofa_totp_last_used_at: nil, twofa_required: false>

rails test test/unit/user_query_test.rb:172

#23 Updated by Marius BALTEANU about 24 hours ago

Go MAEDA wrote:

The test fails with PostgreSQL.

[...]

I've fixed the test in r21860 by adding a new AuthSource and ordering by non null values. I think the problem is generated by how PostgreSQL handles the null values by default.

#24 Updated by Marius BALTEANU about 23 hours ago

I've fixed another issue in r21861 that appears when the column auth source name is added as column and at least one user has an auth source different than internal:

Error:
UsersControllerTest#test_index_with_auth_source_column:
ActionView::Template::Error: undefined method `visible?' for #<AuthSourceLdap id: 1, [...]>

Please let me know what do you think about the fix.

#25 Updated by Jens Krämer about 12 hours ago

That looks good to me. Thanks Marius!

#26 Updated by Marius BALTEANU about 12 hours ago

  • Status changed from Reopened to Closed

Jens Krämer wrote:

That looks good to me. Thanks Marius!

Thanks! All tests pass now, closing this.

Also available in: Atom PDF