Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust User group search to mimic user search #4291

Merged
merged 8 commits into from
Mar 18, 2024
11 changes: 10 additions & 1 deletion app/controllers/api/v1/user_groups_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
39 changes: 27 additions & 12 deletions app/models/user_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
yuenmichelle1 marked this conversation as resolved.
Show resolved Hide resolved
has_one :identity_membership, -> { identity }, class_name: 'Membership'
yuenmichelle1 marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand Down Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions db/migrate/20240216142515_add_tsv_to_user_groups.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class AddTsvToUserGroups < ActiveRecord::Migration[6.1]
yuenmichelle1 marked this conversation as resolved.
Show resolved Hide resolved
def up
add_column :user_groups, :tsv, :tsvector
end

def down
remove_column :user_groups, :tsv
end
end
13 changes: 13 additions & 0 deletions db/migrate/20240216171653_add_index_tsv_to_user_groups.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
class AddIndexTsvToUserGroups < ActiveRecord::Migration[6.1]
yuenmichelle1 marked this conversation as resolved.
Show resolved Hide resolved
# 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
yuenmichelle1 marked this conversation as resolved.
Show resolved Hide resolved
end

def down
remove_index :user_groups, :tsv
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
class CreateSqlTriggerToUpdateTsvVector < ActiveRecord::Migration[6.1]
yuenmichelle1 marked this conversation as resolved.
Show resolved Hide resolved
disable_ddl_transaction!

def up
safety_assured {
yuenmichelle1 marked this conversation as resolved.
Show resolved Hide resolved
execute <<-SQL
yuenmichelle1 marked this conversation as resolved.
Show resolved Hide resolved
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 {
yuenmichelle1 marked this conversation as resolved.
Show resolved Hide resolved
execute <<-SQL
yuenmichelle1 marked this conversation as resolved.
Show resolved Hide resolved
DROP TRIGGER tsvectorupdate
ON user_groups;
SQL
}
end
end
24 changes: 22 additions & 2 deletions db/structure.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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: -
--
Expand Down Expand Up @@ -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
);


Expand Down Expand Up @@ -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: -
--
Expand Down Expand Up @@ -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: -
--
Expand Down Expand Up @@ -4585,6 +4602,9 @@ INSERT INTO "schema_migrations" (version) VALUES
('20211201164326'),
('20221018032140'),
('20230613165746'),
('20231025200957');
('20231025200957'),
('20240216142515'),
('20240216171653'),
('20240216171937');


8 changes: 8 additions & 0 deletions lib/tasks/user_groups.rake
Original file line number Diff line number Diff line change
Expand Up @@ -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
43 changes: 43 additions & 0 deletions spec/controllers/api/v1/user_groups_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
yuenmichelle1 marked this conversation as resolved.
Show resolved Hide resolved
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
yuenmichelle1 marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
Loading