diff --git a/app/controllers/api/v1/user_groups_controller.rb b/app/controllers/api/v1/user_groups_controller.rb index 1234d486b..9fa8194a2 100644 --- a/app/controllers/api/v1/user_groups_controller.rb +++ b/app/controllers/api/v1/user_groups_controller.rb @@ -13,7 +13,16 @@ class Api::V1::UserGroupsController < Api::ApiController allowed_params :update, :name, :stats_visibility, :display_name search_by do |name, query| - query.search_name(name.join(" ")) + search_names = name.join(' ').downcase + display_name_search = query.where('lower(display_name) = ?', search_names) + + if display_name_search.exists? + display_name_search + elsif search_names.present? && search_names.length >= 3 + query.full_search_display_name(search_names) + else + UserGroup.none + end end def destroy_links diff --git a/app/models/user_group.rb b/app/models/user_group.rb index a9a81882b..fab426591 100644 --- a/app/models/user_group.rb +++ b/app/models/user_group.rb @@ -5,19 +5,19 @@ class UserGroup < ApplicationRecord include PgSearch::Model has_many :memberships, dependent: :destroy - has_many :active_memberships, -> { active.not_identity }, class_name: "Membership" - has_one :identity_membership, -> { identity }, class_name: "Membership" + has_many :active_memberships, -> { active.not_identity }, class_name: 'Membership' + has_one :identity_membership, -> { identity }, class_name: 'Membership' has_many :users, through: :memberships has_many :classifications, dependent: :restrict_with_exception has_many :access_control_lists, dependent: :destroy has_many :owned_resources, -> { where("roles && '{owner}'") }, - class_name: "AccessControlList" + class_name: 'AccessControlList' has_many :projects, through: :owned_resources, source: :resource, - source_type: "Project" + source_type: 'Project' has_many :collections, through: :owned_resources, source: :resource, - source_type: "Collection" + source_type: 'Collection' ## # Stats_Visibility Levels (Used for ERAS stats service) @@ -46,23 +46,38 @@ class UserGroup < ApplicationRecord validates :display_name, presence: true validates :name, presence: true, - uniqueness: { case_sensitive: false }, - format: { with: User::USER_LOGIN_REGEX } + uniqueness: { case_sensitive: false }, + format: { with: User::USER_LOGIN_REGEX } - before_validation :default_display_name, on: [:create, :update] + before_validation :default_display_name, on: %i[create update] before_validation :default_join_token, on: [:create] scope :public_groups, -> { where(private: false) } pg_search_scope :search_name, - against: :display_name, - using: :trigram, - ranked_by: ":trigram" + against: [:display_name], + using: { + tsearch: { + dictionary: 'english', + tsvector_column: 'tsv' + } + } + + pg_search_scope :full_search_display_name, + against: [:display_name], + using: { + tsearch: { + dictionary: 'english', + tsvector_column: 'tsv' + }, + trigram: {} + }, + ranked_by: ':tsearch + (0.25 * :trigram)' def self.roles_allowed_to_access(action, klass=nil) roles = case action when :show, :index - [:group_admin, :group_member] + %i[group_admin group_member] else [:group_admin] end diff --git a/db/migrate/20240216142515_add_tsv_to_user_groups.rb b/db/migrate/20240216142515_add_tsv_to_user_groups.rb new file mode 100644 index 000000000..c8c42d29d --- /dev/null +++ b/db/migrate/20240216142515_add_tsv_to_user_groups.rb @@ -0,0 +1,9 @@ +class AddTsvToUserGroups < ActiveRecord::Migration[6.1] + def up + add_column :user_groups, :tsv, :tsvector + end + + def down + remove_column :user_groups, :tsv + end +end diff --git a/db/migrate/20240216171653_add_index_tsv_to_user_groups.rb b/db/migrate/20240216171653_add_index_tsv_to_user_groups.rb new file mode 100644 index 000000000..a74b36571 --- /dev/null +++ b/db/migrate/20240216171653_add_index_tsv_to_user_groups.rb @@ -0,0 +1,13 @@ +class AddIndexTsvToUserGroups < ActiveRecord::Migration[6.1] + # Adding an index non-concurrently blocks writes. Therefore we need to disable ddl transaction + + disable_ddl_transaction! + + def up + add_index :user_groups, :tsv, using: "gin", algorithm: :concurrently + end + + def down + remove_index :user_groups, :tsv + end +end diff --git a/db/migrate/20240216171937_create_sql_trigger_to_update_tsv_vector.rb b/db/migrate/20240216171937_create_sql_trigger_to_update_tsv_vector.rb new file mode 100644 index 000000000..5ac99888e --- /dev/null +++ b/db/migrate/20240216171937_create_sql_trigger_to_update_tsv_vector.rb @@ -0,0 +1,24 @@ +class CreateSqlTriggerToUpdateTsvVector < ActiveRecord::Migration[6.1] + disable_ddl_transaction! + + def up + safety_assured { + execute <<-SQL + CREATE TRIGGER tsvectorupdate BEFORE INSERT OR UPDATE + ON user_groups FOR EACH ROW EXECUTE PROCEDURE + tsvector_update_trigger( + tsv, 'pg_catalog.english', display_name + ); + SQL + } + end + + def down + safety_assured { + execute <<-SQL + DROP TRIGGER tsvectorupdate + ON user_groups; + SQL + } + end +end diff --git a/db/structure.sql b/db/structure.sql index 0c3e5dc97..dcf97baca 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -53,6 +53,8 @@ COMMENT ON EXTENSION pg_trgm IS 'text similarity measurement and index searching SET default_tablespace = ''; +SET default_with_oids = false; + -- -- Name: access_control_lists; Type: TABLE; Schema: public; Owner: - -- @@ -1663,7 +1665,8 @@ CREATE TABLE public.user_groups ( private boolean DEFAULT true NOT NULL, lock_version integer DEFAULT 0, join_token character varying, - stats_visibility integer DEFAULT 0 + stats_visibility integer DEFAULT 0, + tsv tsvector ); @@ -3541,6 +3544,13 @@ CREATE UNIQUE INDEX index_user_groups_on_name ON public.user_groups USING btree CREATE INDEX index_user_groups_on_private ON public.user_groups USING btree (private); +-- +-- Name: index_user_groups_on_tsv; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX index_user_groups_on_tsv ON public.user_groups USING gin (tsv); + + -- -- Name: index_user_project_preferences_on_project_id_and_user_id; Type: INDEX; Schema: public; Owner: - -- @@ -3765,6 +3775,13 @@ CREATE INDEX users_idx_trgm_login ON public.users USING gin (COALESCE((login)::t CREATE TRIGGER tsvectorupdate BEFORE INSERT OR UPDATE ON public.projects FOR EACH ROW EXECUTE PROCEDURE tsvector_update_trigger('tsv', 'pg_catalog.english', 'display_name'); +-- +-- Name: user_groups tsvectorupdate; Type: TRIGGER; Schema: public; Owner: - +-- + +CREATE TRIGGER tsvectorupdate BEFORE INSERT OR UPDATE ON public.user_groups FOR EACH ROW EXECUTE PROCEDURE tsvector_update_trigger('tsv', 'pg_catalog.english', 'display_name'); + + -- -- Name: users tsvectorupdate; Type: TRIGGER; Schema: public; Owner: - -- @@ -4585,6 +4602,9 @@ INSERT INTO "schema_migrations" (version) VALUES ('20211201164326'), ('20221018032140'), ('20230613165746'), -('20231025200957'); +('20231025200957'), +('20240216142515'), +('20240216171653'), +('20240216171937'); diff --git a/lib/tasks/user_groups.rake b/lib/tasks/user_groups.rake index 3b1581c4c..5e1c9c476 100644 --- a/lib/tasks/user_groups.rake +++ b/lib/tasks/user_groups.rake @@ -8,4 +8,12 @@ namespace :user_groups do UserGroup.where(id: user_group_ids_to_update).update_all(stats_visibility: 0) end end + + desc 'Touch user_group updated_at' + task touch_user_group_updated_at: :environment do + UserGroup.select(:id).find_in_batches do |user_groups| + user_group_ids_to_update = user_groups.map(&:id) + UserGroup.where(id: user_group_ids_to_update).update_all(updated_at: Time.current.to_s(:db)) + end + end end diff --git a/spec/controllers/api/v1/user_groups_controller_spec.rb b/spec/controllers/api/v1/user_groups_controller_spec.rb index a1c22ce26..92d50cdfb 100644 --- a/spec/controllers/api/v1/user_groups_controller_spec.rb +++ b/spec/controllers/api/v1/user_groups_controller_spec.rb @@ -49,6 +49,49 @@ end end + describe 'search' do + let(:user_group_with_uniq_name) { create(:user_group, private: false, display_name: 'My Unique Group') } + + before do + # force an update of all user_groups to set the tsv column + user_group_with_uniq_name.reload + user_groups.each(&:reload) + end + + it 'returns exact matched user_group' do + get :index, params: { search: user_group_with_uniq_name.display_name } + expect(json_response[api_resource_name].length).to eq(1) + expect(json_response[api_resource_name][0]['id']).to eq(user_group_with_uniq_name.id.to_s) + end + + it 'returns no user_groups if search is < than 3 chars' do + get :index, params: { search: 'my' } + expect(json_response[api_resource_name].length).to eq(0) + end + + it 'does a full text search on display_name when no exact match' do + get :index, params: { search: 'my uniq' } + expect(json_response[api_resource_name].length).to eq(1) + expect(json_response[api_resource_name][0]['id']).to eq(user_group_with_uniq_name.id.to_s) + end + + it 'does a full text search on display_name on public and accessible user_groups' do + get :index, params: { search: 'group' } + # returns user_group_with_users, user_group_with_uniq_name, the public user_group + expect(json_response[api_resource_name].length).to eq(3) + end + + describe 'as admin' do + it 'does a full text search against display_name on private and public user_groups' do + admin_user = create(:user, admin: true) + default_request scopes: scopes, user_id: admin_user.id + get :index, params: { search: 'group', admin: true } + # returns all 5 user groups + expect(json_response[api_resource_name].length).to eq(5) + end + end + end + context 'with no filters' do it_behaves_like 'is indexable' end