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

AO3-6565 Move admin user search to Elasticsearch, add past emails and username search #4801

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions app/controllers/admin/admin_users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ class Admin::AdminUsersController < Admin::BaseController

def index
authorize User
@role_values = @roles.map{ |role| [role.name.humanize.titlecase, role.name] }
@role = Role.find_by(name: params[:role]) if params[:role]
@users = User.search_by_role(
@role, params[:name], params[:email], params[:user_id],
inactive: params[:inactive], exact: params[:exact], page: params[:page]
)

# Values for the role dropdown:
@role_values = @roles.map { |role| [role.name.humanize.titlecase, role.id] }

@query = UserQuery.new(search_params)
@users = @query.search_results.scope(:with_includes_for_admin_index)
end

def bulk_search
Expand Down Expand Up @@ -177,6 +177,18 @@ def activate
end
end

private

def search_params
allowed_params = if policy(User).can_view_past?
%i[name email role_id user_id inactive page search_past]
else
%i[name email role_id user_id inactive page]
end

params.slice(*allowed_params).permit(*allowed_params)
weeklies marked this conversation as resolved.
Show resolved Hide resolved
end

def log_items
@log_items ||= @user.log_items.sort_by(&:created_at).reverse
end
Expand Down
3 changes: 1 addition & 2 deletions app/models/indexing/scheduled_reindex_job.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class ScheduledReindexJob
MAIN_CLASSES = %w(Pseud Tag Work Bookmark Series ExternalWork).freeze
MAIN_CLASSES = %w[Pseud Tag Work Bookmark Series ExternalWork User].freeze

def self.perform(reindex_type)
classes = case reindex_type
Expand All @@ -16,4 +16,3 @@ def self.run_queue(klass, reindex_type)
end

end

9 changes: 8 additions & 1 deletion app/models/pseud.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,11 @@ class Pseud < ApplicationRecord
validates_length_of :icon_comment_text, allow_blank: true, maximum: ArchiveConfig.ICON_COMMENT_MAX,
too_long: ts("must be less than %{max} characters long.", max: ArchiveConfig.ICON_COMMENT_MAX)

after_create :reindex_user
after_update :check_default_pseud
after_update :expire_caches
after_update :reindex_user, if: :name_changed?
after_destroy :reindex_user
after_commit :reindex_creations, :touch_comments

scope :alphabetical, -> { order(:name) }
Expand Down Expand Up @@ -419,7 +422,7 @@ def delete_icon

def clear_icon
return unless delete_icon?

self.icon = nil unless icon.dirty?
self.icon_alt_text = nil
end
Expand Down Expand Up @@ -449,4 +452,8 @@ def reindex_creations
IndexQueue.enqueue_ids(Bookmark, bookmarks.pluck(:id), :main)
IndexQueue.enqueue_ids(Series, series.pluck(:id), :main)
end

def reindex_user
user.enqueue_to_index
weeklies marked this conversation as resolved.
Show resolved Hide resolved
end
end
9 changes: 7 additions & 2 deletions app/models/roles_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,14 @@ class RolesUser < ApplicationRecord
belongs_to :user
belongs_to :role

delegate :enqueue_to_index, to: :user

after_create :log_role_addition
after_create :enqueue_to_index
after_destroy :log_role_removal
after_destroy :destroy_last_wrangling_activity
after_destroy :enqueue_to_index

def log_role_addition
admin = User.current_user
note = "Change made by #{admin&.login}"
Expand All @@ -12,7 +19,6 @@ def log_role_addition
role_id: role_id })
end

after_destroy :log_role_removal
def log_role_removal
admin = User.current_user
note = "Change made by #{admin&.login}"
Expand All @@ -24,7 +30,6 @@ def log_role_removal

# After removing the tag_wrangler role, remove the
# user's last wrangling activity as well.
after_destroy :destroy_last_wrangling_activity
def destroy_last_wrangling_activity
return unless role.name == "tag_wrangler"

Expand Down
4 changes: 3 additions & 1 deletion app/models/search/indexer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ class Indexer
Tag: %w[TagIndexer],
Pseud: %w[PseudIndexer],
Series: %w[BookmarkedSeriesIndexer],
ExternalWork: %w[BookmarkedExternalWorkIndexer]
ExternalWork: %w[BookmarkedExternalWorkIndexer],
User: %w[UserIndexer]
}.freeze

delegate :klass, :klass_with_includes, :index_name, :document_type, to: :class
Expand All @@ -31,6 +32,7 @@ def self.all
BookmarkIndexer,
PseudIndexer,
TagIndexer,
UserIndexer,
WorkIndexer,
WorkCreatorIndexer
]
Expand Down
70 changes: 70 additions & 0 deletions app/models/search/user_indexer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
class UserIndexer < Indexer
def self.klass
"User"
end

def self.klass_with_includes
User.includes(:pseuds, :roles, :audits)
end

def self.mapping
{
properties: {
login: {
type: "keyword",
normalizer: "keyword_normalizer"
},
email: {
type: "keyword",
normalizer: "keyword_normalizer"
},
names: {
type: "keyword",
normalizer: "keyword_normalizer"
},
all_names: {
type: "keyword",
normalizer: "keyword_normalizer"
},
all_emails: {
type: "keyword",
normalizer: "keyword_normalizer"
}
}
}
end

def self.settings
{
analysis: {
normalizer: {
keyword_normalizer: {
type: "custom",
filter: %w[lowercase asciifolding]
}
}
}
}
end

def document(object)
object.as_json(
root: false,
only: [:id, :login, :email, :created_at],
methods: [:role_ids]
).merge(extra_info(object))
end

def extra_info(object)
names = ([object.login] + object.pseuds.map(&:name)).uniq
past_names = object.historic_values("login")
past_emails = object.historic_values("email")

{
active: object.active?,
names: names,
all_names: (names + past_names).uniq,
all_emails: ([object.email] + past_emails).uniq
}
end
end
55 changes: 55 additions & 0 deletions app/models/search/user_query.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
class UserQuery < Query
def klass
"User"
end

def index_name
UserIndexer.index_name
end

def filters
@filters ||= [
id_filter,
inactive_filter,
role_filter,
email_filter,
name_filter
].flatten.compact
end

def sort
[{ login: { order: :asc } }, { id: { order: :asc } }]
end

################
# FILTERS
################

def id_filter
{ term: { id: options[:user_id] } } if options[:user_id].present?
end

def inactive_filter
{ term: { active: false } } if options[:inactive].present?
end

def role_filter
{ term: { role_ids: options[:role_id] } } if options[:role_id].present?
end

def name_filter
return if options[:name].blank?

field = options[:search_past].present? ? :all_names : :names

{ wildcard: { field => options[:name] } }
end

def email_filter
return if options[:email].blank?

field = options[:search_past].present? ? :all_emails : :email

{ wildcard: { field => options[:email] } }
end
end
55 changes: 14 additions & 41 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ class User < ApplicationRecord
include WorksOwner
include PasswordResetsLimitable
include UserLoggable
include Searchable

devise :database_authenticatable,
:confirmable,
Expand Down Expand Up @@ -251,47 +252,7 @@ def self.for_claims(claims_ids)
where("challenge_claims.id IN (?)", claims_ids)
end

# Find users with a particular role and/or by name, email, and/or id
# Options: inactive, page, exact
def self.search_by_role(role, name, email, user_id, options = {})
return if role.blank? && name.blank? && email.blank? && user_id.blank?

users = User.distinct.order(:login)
if options[:inactive]
users = users.where("confirmed_at IS NULL")
end
if role.present?
users = users.joins(:roles).where("roles.id = ?", role.id)
end
if name.present?
users = users.filter_by_name(name, options[:exact])
end
if email.present?
users = users.filter_by_email(email, options[:exact])
end
if user_id.present?
users = users.where(["users.id = ?", user_id])
end
users.paginate(page: options[:page] || 1)
end

# Scope to look for users by pseud name:
def self.filter_by_name(name, exact)
if exact
joins(:pseuds).where(["pseuds.name = ?", name])
else
joins(:pseuds).where(["pseuds.name LIKE ?", "%#{name}%"])
end
end

# Scope to look for users by email:
def self.filter_by_email(email, exact)
if exact
where(["email = ?", email])
else
where(["email LIKE ?", "%#{email}%"])
end
end
scope :with_includes_for_admin_index, -> { includes(:roles, :fannish_next_of_kin) }

def self.search_multiple_by_email(emails = [])
# Normalise and dedupe emails
Expand Down Expand Up @@ -521,6 +482,18 @@ def reindex_user_creations
IndexQueue.enqueue_ids(Pseud, pseuds.pluck(:id), :main)
end

# Function to make it easier to retrieve info from the audits table.
#
# Looks up all past values of the given field, excluding the current value of
# the field:
def historic_values(field)
field = field.to_s

audits.filter_map do |audit|
audit.audited_changes[field]
end.flatten.uniq.without(self[field])
end

private

# Create and/or return a user account for holding orphaned works
Expand Down
7 changes: 7 additions & 0 deletions app/policies/user_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ class UserPolicy < ApplicationPolicy
# Roles that allow deleting all of a spammer's creations.
SPAM_CLEANUP_ROLES = %w[superadmin policy_and_abuse].freeze

# Roles that allow viewing of past user emails and logins.
VIEW_PAST_USER_INFO_ROLES = %w[superadmin policy_and_abuse open_doors support tag_wrangling].freeze

# Define which roles can update which attributes.
ALLOWED_ATTRIBUTES_BY_ROLES = {
"open_doors" => [roles: []],
Expand Down Expand Up @@ -41,6 +44,10 @@ def can_destroy_spam_creations?
user_has_roles?(SPAM_CLEANUP_ROLES)
end

def can_view_past?
user_has_roles?(VIEW_PAST_USER_INFO_ROLES)
end

def permitted_attributes
ALLOWED_ATTRIBUTES_BY_ROLES.values_at(*user.roles).compact.flatten
end
Expand Down
20 changes: 13 additions & 7 deletions app/views/admin/admin_users/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,24 @@
<%= ts("Search for user with exact ID.") %>
</p>
</dd>
<dt><%= label_tag "role", ts("Role") %></dt>
<dd><%= select_tag "role", "<option></option>".html_safe + options_for_select(@role_values, @role.try(:name)) %></dd>
<dt><%= label_tag "role_id", t(".role") %></dt>
<dd><%= select_tag "role_id", options_for_select(@role_values, params[:role_id].to_i), include_blank: true %></dd>
<dt><%= label_tag "status", ts("Status") %></dt>
<dd>
<%= check_box_tag "inactive", "1", params[:inactive].present? %>
<%= label_tag "inactive", ts("Not yet activated") %>
</dd>
<dt><%= label_tag "settings", ts("Search settings") %></dt>
<dd>
<%= check_box_tag "exact", "1", params[:exact].present? %>
<%= label_tag "exact", ts("Exact match only") %>
</dd>
<% if policy(User).can_view_past? %>
<dt><%= label_tag "settings", t(".settings.label") %></dt>
<dd>
<ul>
<li>
<%= check_box_tag "search_past", "1", params[:search_past].present? %>
<%= label_tag "search_past", t(".settings.past") %>
</li>
</ul>
brianjaustin marked this conversation as resolved.
Show resolved Hide resolved
</dd>
<% end %>
</dl>
<p class="submit"><%= submit_tag ts("Find") %></p>
<% end %>
Expand Down
Loading
Loading