Skip to content

Commit

Permalink
Adjust User group search to mimic user search (#4291)
Browse files Browse the repository at this point in the history
* have user_group search mimic that of user search

* add specs for user_group search functionality and downcase the search_names string

* rubocop fixes of spec

* rubocop fixes on user_group model and user_groups_controller_spec. rename specs with shorter name

* Update user_groups_controller.rb

* remove accidental indentation on up
  • Loading branch information
yuenmichelle1 authored Mar 18, 2024
1 parent 23605b0 commit a637729
Show file tree
Hide file tree
Showing 8 changed files with 156 additions and 15 deletions.
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'
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)
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]
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]
# 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
Original file line number Diff line number Diff line change
@@ -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
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
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
Expand Down

0 comments on commit a637729

Please sign in to comment.