From c950be910e79a5e65feb100e1ba4e85e6ea4ca62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Fri, 8 May 2026 11:30:37 +0200 Subject: [PATCH] Convert custom filters on user administration to standard query --- app/components/table_component.html.erb | 99 ++++++++++--------- app/components/table_component.rb | 1 + .../users/index_sub_header_component.html.erb | 17 +++- .../users/index_sub_header_component.rb | 36 ++++++- ...ent.rb => user_filter_button_component.rb} | 19 ++-- .../users/user_filters_component.rb | 58 +++++++++++ app/controllers/users_controller.rb | 17 +++- .../queries/users/orders/default_order.rb | 2 +- app/models/user_queries/static.rb | 51 ++++++++++ app/models/user_query.rb | 2 + .../user_queries/set_attributes_service.rb | 60 +++++++++++ app/views/users/index.html.erb | 4 +- spec/controllers/users_controller_spec.rb | 31 +++--- spec/features/users/index_spec.rb | 45 +++------ spec/support/pages/admin/users/index.rb | 24 ++++- spec/views/users/index.html.erb_spec.rb | 11 +-- 16 files changed, 344 insertions(+), 133 deletions(-) rename app/components/users/{user_filter_component.rb => user_filter_button_component.rb} (78%) create mode 100644 app/components/users/user_filters_component.rb create mode 100644 app/models/user_queries/static.rb create mode 100644 app/services/user_queries/set_attributes_service.rb diff --git a/app/components/table_component.html.erb b/app/components/table_component.html.erb index 9e1c00c35b1..17e2a3410b9 100644 --- a/app/components/table_component.html.erb +++ b/app/components/table_component.html.erb @@ -26,55 +26,56 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. See COPYRIGHT and LICENSE files for more details. ++#%> - -
-
- <%= render(Primer::BaseComponent.new(**@system_arguments)) do %> - - <% headers.each do |_name, _options| %> - - <% end %> - - - - - <% headers.each do |name, options| %> - <% if sortable_column?(name) %> - <%= helpers.sort_header_tag(name, **options) %> - <% else %> - -
-
- - <%= options[:caption] %> - -
-
- - <% end %> +<%= component_wrapper do %> +
+
+ <%= render(Primer::BaseComponent.new(**@system_arguments)) do %> + + <% headers.each do |_name, _options| %> + <% end %> - - <%# last column for buttons %> -
- - - - - <% if rows.empty? %> - - <%= empty_row_message %> - - <% end %> - <%= render_collection rows %> - - <% end %> - <% if inline_create_link %> -
- <%= inline_create_link %> -
- <% end %> + + + + + <% headers.each do |name, options| %> + <% if sortable_column?(name) %> + <%= helpers.sort_header_tag(name, **options) %> + <% else %> + +
+
+ + <%= options[:caption] %> + +
+
+ + <% end %> + <% end %> + + <%# last column for buttons %> +
+ + + + + <% if rows.empty? %> + + <%= empty_row_message %> + + <% end %> + <%= render_collection rows %> + + <% end %> + <% if inline_create_link %> +
+ <%= inline_create_link %> +
+ <% end %> +
-
-<% if paginated? %> - <%= helpers.pagination_links_full rows %> + <% if paginated? %> + <%= helpers.pagination_links_full rows %> + <% end %> <% end %> diff --git a/app/components/table_component.rb b/app/components/table_component.rb index efe7f01ebd8..470bafc59c6 100644 --- a/app/components/table_component.rb +++ b/app/components/table_component.rb @@ -32,6 +32,7 @@ # Abstract view component. Subclass this for a concrete table. class TableComponent < ApplicationComponent include Primer::AttributesHelper + include OpTurbo::Streamable def initialize(rows: [], table_arguments: {}, **) super(rows, **) diff --git a/app/components/users/index_sub_header_component.html.erb b/app/components/users/index_sub_header_component.html.erb index acad7b63cc4..87e32613d84 100644 --- a/app/components/users/index_sub_header_component.html.erb +++ b/app/components/users/index_sub_header_component.html.erb @@ -1,5 +1,5 @@ <%= - render(Primer::OpenProject::SubHeader.new) do |subheader| + render(Primer::OpenProject::SubHeader.new(data: sub_header_data_attributes)) do |subheader| subheader.with_action_button( scheme: :primary, leading_icon: :plus, @@ -10,8 +10,21 @@ t("activerecord.models.user") end + subheader.with_filter_input( + name: "any_name_attribute", + label: t(:label_search), + value: filter_input_value, + placeholder: t(:label_search), + clear_button_id: clear_button_id, + data: filter_input_data_attributes + ) + + subheader.with_filter_component do + render Users::UserFilterButtonComponent.new(query: @query) + end + subheader.with_bottom_pane_component do - render Users::UserFilterComponent.new(@params, groups: @groups, status: @status) + render Users::UserFiltersComponent.new(query: @query, initially_expanded: filters_expanded?) end end %> diff --git a/app/components/users/index_sub_header_component.rb b/app/components/users/index_sub_header_component.rb index 8f4ed16d22c..700a233ee95 100644 --- a/app/components/users/index_sub_header_component.rb +++ b/app/components/users/index_sub_header_component.rb @@ -32,11 +32,39 @@ module Users class IndexSubHeaderComponent < ApplicationComponent include ApplicationHelper - def initialize(groups:, status:, params:) + def initialize(query:) super - @groups = groups - @status = status - @params = params + @query = query + end + + def filter_input_value + @query.find_active_filter(:any_name_attribute)&.values&.first + end + + def sub_header_data_attributes + { + controller: "filter--filters-form", + "filter--filters-form-perform-turbo-requests-value": true, + "filter--filters-form-clear-button-id-value": clear_button_id, + "filter--filters-form-display-filters-value": filters_expanded? + } + end + + def filter_input_data_attributes + { + "filter-name": "any_name_attribute", + "filter-type": "string", + "filter-operator": "~", + "filter--filters-form-target": "simpleFilter filterValueContainer simpleValue" + } + end + + def clear_button_id + "user-filters-form-clear-button" + end + + def filters_expanded? + params[:filters].present? end end end diff --git a/app/components/users/user_filter_component.rb b/app/components/users/user_filter_button_component.rb similarity index 78% rename from app/components/users/user_filter_component.rb rename to app/components/users/user_filter_button_component.rb index ed4242adfa2..a055d4402a1 100644 --- a/app/components/users/user_filter_component.rb +++ b/app/components/users/user_filter_button_component.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -#-- copyright +# -- copyright # OpenProject is an open source project management software. # Copyright (C) the OpenProject GmbH # @@ -26,16 +26,19 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. # # See COPYRIGHT and LICENSE files for more details. -#++ +# ++ module Users - class UserFilterComponent < ::UserFilterComponent - def filter_role(query, role_id) - super.uniq - end + class UserFilterButtonComponent < Filter::FilterButtonComponent + HIDDEN_FILTERS = [ + Queries::Users::Filters::AnyNameAttributeFilter, + Queries::Users::Filters::BlockedFilter + ].freeze - def clear_url - users_path + private + + def filters_count + query.filters.count { |f| HIDDEN_FILTERS.none? { |klass| f.is_a?(klass) } } end end end diff --git a/app/components/users/user_filters_component.rb b/app/components/users/user_filters_component.rb new file mode 100644 index 00000000000..912b49e9f60 --- /dev/null +++ b/app/components/users/user_filters_component.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +# -- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +# ++ + +module Users + class UserFiltersComponent < Filter::FilterComponent + def turbo_requests? = true + + def allowed_filters + super + .grep_v(Queries::Users::Filters::AnyNameAttributeFilter) + .grep_v(Queries::Users::Filters::BlockedFilter) + .sort_by(&:human_name) + end + + protected + + def additional_filter_attributes(filter) + case filter + when Queries::Users::Filters::GroupFilter + { + autocomplete_options: { + component: "opce-group-autocompleter", + resource: "groups" + } + } + else + super + end + end + end +end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index a2efeb23fc4..a281c75a35b 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -79,11 +79,15 @@ class UsersController < ApplicationController include SortHelper include CustomFieldsHelper include PaginationHelper + include Queries::Loading + + before_action :load_query_or_deny_access, only: :index def index - @groups = Group.visible.sort - @status = Users::UserFilterComponent.status_param params - @users = Users::UserFilterComponent.filter params + respond_to do |format| + format.html + format.turbo_stream { render_index_turbo_stream } + end end def show @@ -423,6 +427,13 @@ class UsersController < ApplicationController status: User.statuses[:invited]) end + def render_index_turbo_stream + update_via_turbo_stream(component: Users::UserFilterButtonComponent.new(query: @query)) + replace_via_turbo_stream(component: Users::TableComponent.new(rows: @query, current_user:)) + turbo_streams << turbo_stream.push_state(url_for(params.permit(:filters, :sortBy, :sort, :page, :per_page))) + render turbo_stream: turbo_streams + end + def prepare_views_for_tab # rubocop:disable Metrics/AbcSize if params[:tab] == "non_working_times" authorize_manage_working_times diff --git a/app/models/queries/users/orders/default_order.rb b/app/models/queries/users/orders/default_order.rb index 7a30b6e0ae4..64c55796eb0 100644 --- a/app/models/queries/users/orders/default_order.rb +++ b/app/models/queries/users/orders/default_order.rb @@ -32,6 +32,6 @@ class Queries::Users::Orders::DefaultOrder < Queries::Orders::Base self.model = User def self.key - /\A(id|lastname|firstname|mail|login)\z/ + /\A(id|lastname|firstname|mail|login|admin|created_at|last_login_on)\z/ end end diff --git a/app/models/user_queries/static.rb b/app/models/user_queries/static.rb new file mode 100644 index 00000000000..24a535072d5 --- /dev/null +++ b/app/models/user_queries/static.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +# -- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +# ++ + +class UserQueries::Static + DEFAULT = "active" + + class << self + def query(id) + case id + when DEFAULT, nil + static_query_active + end + end + + private + + def static_query_active + UserQuery.new(name: I18n.t(:status_active)) do |query| + query.where("status", "=", "active") + query.clear_changes_information + end + end + end +end diff --git a/app/models/user_query.rb b/app/models/user_query.rb index 19c4c32ce60..d8678d3d2a5 100644 --- a/app/models/user_query.rb +++ b/app/models/user_query.rb @@ -29,6 +29,8 @@ #++ class UserQuery < PersistedQuery + scope :visible, ->(user = User.current) { where(principal: user) } + def self.model User end diff --git a/app/services/user_queries/set_attributes_service.rb b/app/services/user_queries/set_attributes_service.rb new file mode 100644 index 00000000000..513ee3c778a --- /dev/null +++ b/app/services/user_queries/set_attributes_service.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +# -- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +# ++ + +class UserQueries::SetAttributesService < BaseServices::SetAttributes + private + + def set_attributes(params) + set_filters(params.delete(:filters)) + set_order(params.delete(:orders)) + + super + end + + def set_default_attributes(_params) + # No user or project association needed for admin-scoped user queries + end + + def set_filters(filters) + return unless filters + + model.filters.clear + filters.each do |filter| + model.where(filter[:attribute], filter[:operator], filter[:values]) + end + end + + def set_order(orders) + return unless orders + + model.orders.clear + model.order(orders.to_h { |o| [o[:attribute], o[:direction]] }) + end +end diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb index d7afd805ede..1463feec0a1 100644 --- a/app/views/users/index.html.erb +++ b/app/views/users/index.html.erb @@ -30,6 +30,6 @@ See COPYRIGHT and LICENSE files for more details. <%= render Users::IndexPageHeaderComponent.new %> -<%= render Users::IndexSubHeaderComponent.new(groups: @groups, status: @status, params: params) %> +<%= render Users::IndexSubHeaderComponent.new(query: @query) %> -<%= render Users::TableComponent.new(rows: @users, current_user:) %> +<%= render Users::TableComponent.new(rows: @query, current_user:) %> diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 2953024bd74..aadb1bacaee 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -736,15 +736,15 @@ RSpec.describe UsersController do expect(response).to have_rendered("index") end - it "assigns users" do - expect(assigns(:users)).to contain_exactly(user, admin, user_manager) + it "assigns a query" do + expect(assigns(:query).results).to contain_exactly(user, admin, user_manager) end context "with a name filter" do - let(:params) { { name: user.firstname } } + let(:params) { { filters: %(any_name_attribute ~ "#{user.firstname}") } } - it "assigns users" do - expect(assigns(:users)).to contain_exactly(user) + it "assigns a query filtered by name" do + expect(assigns(:query).results).to contain_exactly(user) end end @@ -752,11 +752,11 @@ RSpec.describe UsersController do let(:group) { create(:group, members: [user]) } let(:params) do - { group_id: group.id } + { filters: %(group = "#{group.id}") } end - it "assigns users" do - expect(assigns(:users)).to contain_exactly(user) + it "assigns a query filtered by group" do + expect(assigns(:query).results).to contain_exactly(user) end end @@ -764,7 +764,7 @@ RSpec.describe UsersController do let!(:deleted_user) { create(:user_marked_for_deletion) } it "does not include this user to the users list" do - expect(assigns(:users)).to contain_exactly(user, admin, user_manager) + expect(assigns(:query).results).to contain_exactly(user, admin, user_manager) end end end @@ -806,14 +806,13 @@ RSpec.describe UsersController do context "enabled" do before do - allow(Setting).to receive(:session_ttl_enabled?).and_return(true) - allow(Setting).to receive(:session_ttl).and_return("120") + allow(Setting).to receive_messages(session_ttl_enabled?: true, session_ttl: "120") @controller.send(:logged_user=, admin) end context "before 120 min of inactivity" do before do - session[:updated_at] = Time.now - 1.hour + session[:updated_at] = 1.hour.ago get :index end @@ -822,7 +821,7 @@ RSpec.describe UsersController do context "after 120 min of inactivity" do before do - session[:updated_at] = Time.now - 3.hours + session[:updated_at] = 3.hours.ago get :index end @@ -842,7 +841,7 @@ RSpec.describe UsersController do context "with ttl = 0" do before do allow(Setting).to receive(:session_ttl).and_return("0") - session[:updated_at] = Time.now - 1.hour + session[:updated_at] = 1.hour.ago get :index end @@ -852,7 +851,7 @@ RSpec.describe UsersController do context "with ttl < 0" do before do allow(Setting).to receive(:session_ttl).and_return("-60") - session[:updated_at] = Time.now - 1.hour + session[:updated_at] = 1.hour.ago get :index end @@ -862,7 +861,7 @@ RSpec.describe UsersController do context "with ttl < 5 > 0" do before do allow(Setting).to receive(:session_ttl).and_return("4") - session[:updated_at] = Time.now - 1.hour + session[:updated_at] = 1.hour.ago get :index end diff --git a/spec/features/users/index_spec.rb b/spec/features/users/index_spec.rb index 2841c514245..b54bb07eb0d 100644 --- a/spec/features/users/index_spec.rb +++ b/spec/features/users/index_spec.rb @@ -68,27 +68,18 @@ RSpec.describe "index users", :js do shared_let(:registered_user) { create(:user, status: User.statuses[:registered]) } shared_let(:invited_user) { create(:user, status: User.statuses[:invited]) } - it "shows the users by status and allows status manipulations", + it "shows active users by default and allows status filtering and manipulations", with_settings: { brute_force_block_after_failed_logins: 5, brute_force_block_minutes: 10 } do index_page.visit! - # Order is by id, asc - # so first ones created are on top. - index_page.expect_listed(current_user, active_user, registered_user, invited_user) - - index_page.order_by("Created on") - index_page.expect_order(invited_user, registered_user, active_user, current_user) - - index_page.order_by("Created on") - index_page.expect_order(current_user, active_user, registered_user, invited_user) + # Default filter: active users only + index_page.expect_listed(current_user, active_user) index_page.lock_user(active_user) - index_page.expect_listed(current_user, active_user, registered_user, invited_user) + # active_user is now locked — still visible until filter changes index_page.expect_user_locked(active_user) - - expect(active_user.reload) - .to be_locked + expect(active_user.reload).to be_locked index_page.filter_by_status("locked permanently") index_page.expect_listed(active_user) @@ -106,30 +97,19 @@ RSpec.describe "index users", :js do index_page.filter_by_name(active_user.lastname[0..-3]) index_page.expect_listed(active_user) - # temporarily block user + # temporarily block user — reset via action, no filter needed active_user.update(failed_login_count: 6, last_failed_login_on: 9.minutes.ago) index_page.clear_filters - index_page.expect_listed(current_user, active_user, registered_user, invited_user) - - index_page.filter_by_status("locked temporarily") - index_page.expect_listed(active_user) + # after clear, default active filter is restored + index_page.expect_listed(current_user, active_user) index_page.reset_failed_logins(active_user) - index_page.expect_non_listed - - # temporarily block user and lock permanently - active_user.reload - active_user.update(failed_login_count: 6, - last_failed_login_on: 9.minutes.ago) - index_page.clear_filters - - index_page.filter_by_status("locked temporarily") - index_page.expect_listed(active_user) + # still listed — reset doesn't change status + index_page.expect_listed(current_user, active_user) + # lock permanently and unlock index_page.lock_user(active_user) - index_page.expect_listed(active_user) - index_page.filter_by_status("locked permanently") index_page.expect_listed(active_user) @@ -145,7 +125,6 @@ RSpec.describe "index users", :js do index_page.activate_user(registered_user) index_page.filter_by_status("active") - index_page.expect_listed(current_user, active_user, registered_user) end @@ -155,7 +134,7 @@ RSpec.describe "index users", :js do it "can too visit the page" do index_page.visit! - index_page.expect_listed(admin, current_user, active_user, registered_user, invited_user) + index_page.expect_listed(admin, current_user, active_user) end end end diff --git a/spec/support/pages/admin/users/index.rb b/spec/support/pages/admin/users/index.rb index b446bf3a717..d79d9fad239 100644 --- a/spec/support/pages/admin/users/index.rb +++ b/spec/support/pages/admin/users/index.rb @@ -62,25 +62,39 @@ module Pages end def filter_by_status(value) - select value, from: "Status:" - click_button "Apply" + open_filter_panel + select "Status", from: "Add filter" + within_filter("status") do + find("[data-filter-name='status'] select").select(value) + end wait_for_network_idle end def filter_by_name(value) - fill_in "Name", with: value - click_button "Apply" + fill_in "Search", with: value wait_for_network_idle end def clear_filters - click_link "Clear" + find_by_id("user-filters-form-clear-button").click wait_for_network_idle end + def open_filter_panel + find("[data-test-selector='filter-component-toggle']").click unless filter_panel_open? + end + + def filter_panel_open? + page.has_css?(".advanced-filters--container.-expanded", wait: 0) + end + + def within_filter(name, &) + within("[data-filter-name='#{name}']", &) + end + def order_by(key) within "thead" do click_link key diff --git a/spec/views/users/index.html.erb_spec.rb b/spec/views/users/index.html.erb_spec.rb index ff8d74da9b5..b10886832cc 100644 --- a/spec/views/users/index.html.erb_spec.rb +++ b/spec/views/users/index.html.erb_spec.rb @@ -39,9 +39,7 @@ RSpec.describe "users/index" do before do User.system # create system user which is active but should not count towards limit - assign(:users, User.where(id: [admin.id, user.id])) - assign(:status, "all") - assign(:groups, Group.visible) + assign(:query, UserQueries::Static.query(nil)) without_partial_double_verification do allow(view).to receive_messages(current_user: admin, controller_name: "users", action_name: "index") @@ -50,13 +48,6 @@ RSpec.describe "users/index" do subject { rendered.squish } - it "renders the user table" do - render - - expect(subject).to have_text("#{admin.firstname} #{admin.lastname}") - expect(subject).to have_text("Scarlet Scallywag") - end - context "with an Enterprise token" do before do create_enterprise_token("token_5_users", restrictions: { active_user_count: 5 })