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

@@ -94,8 +94,8 @@ module Admin
         :local,
         :remote,
         :by_domain,
+        :active,
         :silenced,
-        :alphabetic,
         :suspended,
         :username,
         :display_name,

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

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

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')

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]');

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}%")) }

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

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
 

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
+      \-

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
 

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')