From 73faadad28b897cd61cc1d6c8bee47e7d72a51a1 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 26 Nov 2018 15:53:27 +0100 Subject: Redesign admin accounts index (#9340) * Improve overview of accounts in admin UI - Display suspended status, role, last activity and IP prominently - Default to showing local accounts - Default to not showing suspended accounts * Remove unused strings * Fix tests * Allow filtering accounts by IP mask --- app/controllers/admin/accounts_controller.rb | 2 +- .../admin/account_moderation_notes_helper.rb | 2 +- app/helpers/admin/filter_helper.rb | 2 +- app/helpers/stream_entries_helper.rb | 8 +++-- app/javascript/packs/public.js | 2 +- app/models/account.rb | 1 + app/models/account_filter.rb | 23 ++++++------- app/models/user.rb | 1 - app/views/admin/accounts/_account.html.haml | 23 ++++++------- app/views/admin/accounts/index.html.haml | 38 +++++----------------- app/views/admin/accounts/show.html.haml | 2 +- 11 files changed, 41 insertions(+), 63 deletions(-) (limited to 'app') diff --git a/app/controllers/admin/accounts_controller.rb b/app/controllers/admin/accounts_controller.rb index 5d57fe361..f155543ce 100644 --- a/app/controllers/admin/accounts_controller.rb +++ b/app/controllers/admin/accounts_controller.rb @@ -94,8 +94,8 @@ module Admin :local, :remote, :by_domain, + :active, :silenced, - :alphabetic, :suspended, :username, :display_name, diff --git a/app/helpers/admin/account_moderation_notes_helper.rb b/app/helpers/admin/account_moderation_notes_helper.rb index 4d8f0352e..40b2a5289 100644 --- a/app/helpers/admin/account_moderation_notes_helper.rb +++ b/app/helpers/admin/account_moderation_notes_helper.rb @@ -24,7 +24,7 @@ module Admin::AccountModerationNotesHelper def name_tag_classes(account, inline = false) classes = [inline ? 'inline-name-tag' : 'name-tag'] - classes << 'suspended' if account.suspended? + classes << 'suspended' if account.suspended? || (account.local? && account.user.nil?) classes.join(' ') end end diff --git a/app/helpers/admin/filter_helper.rb b/app/helpers/admin/filter_helper.rb index 60e5142e3..9a663051c 100644 --- a/app/helpers/admin/filter_helper.rb +++ b/app/helpers/admin/filter_helper.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Admin::FilterHelper - ACCOUNT_FILTERS = %i(local remote by_domain silenced suspended alphabetic username display_name email ip staff).freeze + ACCOUNT_FILTERS = %i(local remote by_domain active silenced suspended username display_name email ip staff).freeze REPORT_FILTERS = %i(resolved account_id target_account_id).freeze INVITE_FILTER = %i(available expired).freeze CUSTOM_EMOJI_FILTERS = %i(local remote by_domain shortcode).freeze diff --git a/app/helpers/stream_entries_helper.rb b/app/helpers/stream_entries_helper.rb index ac655f622..033d435c4 100644 --- a/app/helpers/stream_entries_helper.rb +++ b/app/helpers/stream_entries_helper.rb @@ -34,12 +34,14 @@ module StreamEntriesHelper end end - def account_badge(account) + def account_badge(account, all: false) if account.bot? content_tag(:div, content_tag(:div, t('accounts.roles.bot'), class: 'account-role bot'), class: 'roles') - elsif Setting.show_staff_badge && account.user_staff? + elsif (Setting.show_staff_badge && account.user_staff?) || all content_tag(:div, class: 'roles') do - if account.user_admin? + if all && !account.user_staff? + content_tag(:div, t('admin.accounts.roles.user'), class: 'account-role') + elsif account.user_admin? content_tag(:div, t('accounts.roles.admin'), class: 'account-role admin') elsif account.user_moderator? content_tag(:div, t('accounts.roles.moderator'), class: 'account-role moderator') diff --git a/app/javascript/packs/public.js b/app/javascript/packs/public.js index 3b02b7c39..36b1fd26b 100644 --- a/app/javascript/packs/public.js +++ b/app/javascript/packs/public.js @@ -63,7 +63,7 @@ function main() { content.textContent = timeAgoString({ formatMessage: ({ id, defaultMessage }, values) => (new IntlMessageFormat(messages[id] || defaultMessage, locale)).format(values), formatDate: (date, options) => (new Intl.DateTimeFormat(locale, options)).format(date), - }, datetime, now, datetime.getFullYear()); + }, datetime, now, now.getFullYear()); }); const reactComponents = document.querySelectorAll('[data-component]'); diff --git a/app/models/account.rb b/app/models/account.rb index 593ee29f7..46d32a36e 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -123,6 +123,7 @@ class Account < ApplicationRecord scope :suspended, -> { where(suspended: true) } scope :without_suspended, -> { where(suspended: false) } scope :recent, -> { reorder(id: :desc) } + scope :bots, -> { where(actor_type: %w(Application Service)) } scope :alphabetic, -> { order(domain: :asc, username: :asc) } scope :by_domain_accounts, -> { group(:domain).select(:domain, 'COUNT(*) AS accounts_count').order('accounts_count desc') } scope :matches_username, ->(value) { where(arel_table[:username].matches("#{value}%")) } diff --git a/app/models/account_filter.rb b/app/models/account_filter.rb index 84364bf1b..b10f50db7 100644 --- a/app/models/account_filter.rb +++ b/app/models/account_filter.rb @@ -5,13 +5,14 @@ class AccountFilter def initialize(params) @params = params + set_defaults! end def results - scope = Account.recent + scope = Account.recent.includes(:user) params.each do |key, value| - scope.merge!(scope_for(key, value)) if value.present? + scope.merge!(scope_for(key, value.to_s.strip)) if value.present? end scope @@ -19,6 +20,11 @@ class AccountFilter private + def set_defaults! + params['local'] = '1' if params['remote'].blank? + params['active'] = '1' if params['suspended'].blank? && params['silenced'].blank? + end + def scope_for(key, value) case key.to_s when 'local' @@ -27,10 +33,10 @@ class AccountFilter Account.remote when 'by_domain' Account.where(domain: value) + when 'active' + Account.without_suspended when 'silenced' Account.silenced - when 'alphabetic' - Account.reorder(nil).alphabetic when 'suspended' Account.suspended when 'username' @@ -40,11 +46,7 @@ class AccountFilter when 'email' accounts_with_users.merge User.matches_email(value) when 'ip' - if valid_ip?(value) - accounts_with_users.merge User.with_recent_ip_address(value) - else - Account.default_scoped - end + valid_ip?(value) ? accounts_with_users.where('users.current_sign_in_ip <<= ?', value) : Account.none when 'staff' accounts_with_users.merge User.staff else @@ -57,8 +59,7 @@ class AccountFilter end def valid_ip?(value) - IPAddr.new(value) - true + IPAddr.new(value) && true rescue IPAddr::InvalidAddressError false end diff --git a/app/models/user.rb b/app/models/user.rb index 69fa0688a..453ffa8b0 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -83,7 +83,6 @@ class User < ApplicationRecord scope :inactive, -> { where(arel_table[:current_sign_in_at].lt(ACTIVE_DURATION.ago)) } scope :active, -> { confirmed.where(arel_table[:current_sign_in_at].gteq(ACTIVE_DURATION.ago)).joins(:account).where(accounts: { suspended: false }) } scope :matches_email, ->(value) { where(arel_table[:email].matches("#{value}%")) } - scope :with_recent_ip_address, ->(value) { where(arel_table[:current_sign_in_ip].eq(value).or(arel_table[:last_sign_in_ip].eq(value))) } before_validation :sanitize_languages diff --git a/app/views/admin/accounts/_account.html.haml b/app/views/admin/accounts/_account.html.haml index c6e63152d..0fadaae1e 100644 --- a/app/views/admin/accounts/_account.html.haml +++ b/app/views/admin/accounts/_account.html.haml @@ -1,18 +1,15 @@ %tr - %td.username - = account.username %td - - unless account.local? - = link_to account.domain, admin_accounts_path(by_domain: account.domain) + = admin_account_link_to(account) %td - - if account.local? - - if account.user.nil? - = t("admin.accounts.moderation.suspended") - - else - = t("admin.accounts.roles.#{account.user.role}") + %div{ style: 'margin: -2px 0' }= account_badge(account, all: true) + %td + - if account.user_current_sign_in_ip + %samp= account.user_current_sign_in_ip - else - = account.protocol.humanize + \- %td - = table_link_to 'circle', t('admin.accounts.web'), web_path("accounts/#{account.id}") - = table_link_to 'globe', t('admin.accounts.public'), TagManager.instance.url_for(account) - = table_link_to 'pencil', t('admin.accounts.edit'), admin_account_path(account.id) + - if account.user_current_sign_in_at + %time.time-ago{ datetime: account.user_current_sign_in_at.iso8601, title: l(account.user_current_sign_in_at) }= l account.user_current_sign_in_at + - else + \- diff --git a/app/views/admin/accounts/index.html.haml b/app/views/admin/accounts/index.html.haml index 4bee73adc..0d31eee36 100644 --- a/app/views/admin/accounts/index.html.haml +++ b/app/views/admin/accounts/index.html.haml @@ -5,41 +5,19 @@ .filter-subset %strong= t('admin.accounts.location.title') %ul - %li= filter_link_to t('admin.accounts.location.all'), local: nil, remote: nil - %li - - if selected? local: '1', remote: nil - = filter_link_to t('admin.accounts.location.local'), {local: nil, remote: nil}, {local: '1', remote: nil} - - else - = filter_link_to t('admin.accounts.location.local'), local: '1', remote: nil - %li - - if selected? remote: '1', local: nil - = filter_link_to t('admin.accounts.location.remote'), {remote: nil, local: nil}, {remote: '1', local: nil} - - else - = filter_link_to t('admin.accounts.location.remote'), remote: '1', local: nil + %li= filter_link_to t('admin.accounts.location.local'), remote: nil + %li= filter_link_to t('admin.accounts.location.remote'), remote: '1' .filter-subset %strong= t('admin.accounts.moderation.title') %ul - %li= filter_link_to t('admin.accounts.moderation.all'), silenced: nil, suspended: nil - %li - - if selected? silenced: '1' - = filter_link_to t('admin.accounts.moderation.silenced'), {silenced: nil}, {silenced: '1'} - - else - = filter_link_to t('admin.accounts.moderation.silenced'), silenced: '1' - %li - - if selected? suspended: '1' - = filter_link_to t('admin.accounts.moderation.suspended'), {suspended: nil}, {suspended: '1'} - - else - = filter_link_to t('admin.accounts.moderation.suspended'), suspended: '1' + %li= filter_link_to t('admin.accounts.moderation.active'), silenced: nil, suspended: nil + %li= filter_link_to t('admin.accounts.moderation.silenced'), silenced: '1', suspended: nil + %li= filter_link_to t('admin.accounts.moderation.suspended'), suspended: '1', silenced: nil .filter-subset %strong= t('admin.accounts.role') %ul %li= filter_link_to t('admin.accounts.moderation.all'), staff: nil %li= filter_link_to t('admin.accounts.roles.staff'), staff: '1' - .filter-subset - %strong= t('admin.accounts.order.title') - %ul - %li= filter_link_to t('admin.accounts.order.most_recent'), alphabetic: nil - %li= filter_link_to t('admin.accounts.order.alphabetic'), alphabetic: '1' = form_tag admin_accounts_url, method: 'GET', class: 'simple_form' do .fields-group @@ -60,9 +38,9 @@ %thead %tr %th= t('admin.accounts.username') - %th= t('admin.accounts.domain') - %th - %th + %th= t('admin.accounts.role') + %th= t('admin.accounts.most_recent_ip') + %th= t('admin.accounts.most_recent_activity') %tbody = render @accounts diff --git a/app/views/admin/accounts/show.html.haml b/app/views/admin/accounts/show.html.haml index 17f1f224d..c1a5fc1bd 100644 --- a/app/views/admin/accounts/show.html.haml +++ b/app/views/admin/accounts/show.html.haml @@ -68,7 +68,7 @@ %time.formatted{ datetime: @account.user_current_sign_in_at.iso8601, title: l(@account.user_current_sign_in_at) } = l @account.user_current_sign_in_at - else - Never + \- - else %tr %th= t('admin.accounts.profile_url') -- cgit