Merge branch...

Merge branch 'ee-43681-display-last-activity-and-created-at-datetimes-for-users-in-admin-users' into 'master'

EE Resolve "Display last activity and created at datetimes for users"

Closes gitlab-ce#43681

See merge request gitlab-org/gitlab-ee!9100

app/helpers/sorting_helper.rb

@@ -130,7 +130,9 @@ module SortingHelper
       sort_value_recently_created => sort_title_recently_created,
       sort_value_oldest_created => sort_title_oldest_created,
       sort_value_recently_updated => sort_title_recently_updated,
-      sort_value_oldest_updated => sort_title_oldest_updated
+      sort_value_oldest_updated => sort_title_oldest_updated,
+      sort_value_recently_last_activity => sort_title_recently_last_activity,
+      sort_value_oldest_last_activity => sort_title_oldest_last_activity
     }
   end
 
@@ -319,6 +321,14 @@ module SortingHelper
     s_('SortOptions|Most stars')
   end
 
+  def sort_title_oldest_last_activity
+    s_('SortOptions|Oldest last activity')
+  end
+
+  def sort_title_recently_last_activity
+    s_('SortOptions|Recent last activity')
+  end
+
   # Values.
   def sort_value_access_level_asc
     'access_level_asc'
@@ -447,4 +457,12 @@ module SortingHelper
   def sort_value_most_stars
     'stars_desc'
   end
+
+  def sort_value_oldest_last_activity
+    'last_activity_on_asc'
+  end
+
+  def sort_value_recently_last_activity
+    'last_activity_on_desc'
+  end
 end

app/helpers/users_helper.rb

@@ -74,6 +74,15 @@ module UsersHelper
     Gitlab.config.gitlab.impersonation_enabled
   end
 
+  def user_badges_in_admin_section(user)
+    [].tap do |badges|
+      badges << { text: s_('AdminUsers|Blocked'), variant: 'danger' } if user.blocked?
+      badges << { text: s_('AdminUsers|Admin'), variant: 'success' } if user.admin?
+      badges << { text: s_('AdminUsers|External'), variant: 'secondary' } if user.external?
+      badges << { text: s_("AdminUsers|It's you!"), variant: nil } if current_user == user
+    end
+  end
+
   private
 
   def get_profile_tabs

app/models/user.rb

@@ -267,6 +267,8 @@ class User < ApplicationRecord
   scope :without_projects, -> { joins('LEFT JOIN project_authorizations ON users.id = project_authorizations.user_id').where(project_authorizations: { user_id: nil }) }
   scope :order_recent_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'DESC')) }
   scope :order_oldest_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'ASC')) }
+  scope :order_recent_last_activity, -> { reorder(Gitlab::Database.nulls_last_order('last_activity_on', 'DESC')) }
+  scope :order_oldest_last_activity, -> { reorder(Gitlab::Database.nulls_first_order('last_activity_on', 'ASC')) }
   scope :confirmed, -> { where.not(confirmed_at: nil) }
   scope :by_username, -> (usernames) { iwhere(username: Array(usernames).map(&:to_s)) }
   scope :for_todos, -> (todos) { where(id: todos.select(:user_id)) }
@@ -337,6 +339,8 @@ class User < ApplicationRecord
       case order_method.to_s
       when 'recent_sign_in' then order_recent_sign_in
       when 'oldest_sign_in' then order_oldest_sign_in
+      when 'last_activity_on_desc' then order_recent_last_activity
+      when 'last_activity_on_asc' then order_oldest_last_activity
       else
         order_by(order_method)
       end

app/views/admin/users/_user.html.haml

-%li.flex-row
-  .user-avatar
-    = image_tag avatar_icon_for_user(user), class: "avatar", alt: ''
-  .row-main-content
-    .user-name.row-title.str-truncated-100
-      = link_to user.name, [:admin, user], class: "js-user-link", data: { user_id: user.id }
-    - if user.blocked?
-      %span.badge.badge-danger blocked
-    - if user.note.present?
-      = link_to "#", { "data-toggle" => "tooltip", title: user.note, class: "user-note"} do
-        = icon("sticky-note-o cgrey")
-    - if user.admin?
-      %span.badge.badge-success Admin
-    - if user.external?
-      %span.badge.badge-secondary External
-    - if user == current_user
-      %span It's you!
-    .row-second-line.str-truncated-100
-      = mail_to user.email, user.email
-  .controls
-
-    = render 'admin/namespace_plan_badge', namespace: user.namespace
-
-    = link_to 'Edit', edit_admin_user_path(user), id: "edit_#{dom_id(user)}", class: 'btn'
+.gl-responsive-table-row{ role: 'row' }
+  .table-section.section-40
+    .table-mobile-header{ role: 'rowheader' }
+      = _('Name')
+    .table-mobile-content
+      = render 'user_detail', user: user
+  .table-section.section-25
+    .table-mobile-header{ role: 'rowheader' }
+      = _('Created on')
+    .table-mobile-content
+      = l(user.created_at.to_date, format: :admin)
+  .table-section.section-15
+    .table-mobile-header{ role: 'rowheader' }
+      = _('Last activity')
+    .table-mobile-content
+      = user.last_activity_on.nil? ? _('Never') : l(user.last_activity_on, format: :admin)
+  .table-section.section-20.table-button-footer
+    .table-action-buttons
+      = link_to _('Edit'), edit_admin_user_path(user), id: "edit_#{dom_id(user)}", class: 'btn btn-default'
       - unless user == current_user
-      .dropdown.inline
-        %a.dropdown-new.btn.btn-default#project-settings-button{ href: '#', data: { toggle: 'dropdown' } }
+        %button.dropdown-new.btn.btn-default{ type: 'button', data: { toggle: 'dropdown' } }
           = icon('cog')
           = icon('caret-down')
         %ul.dropdown-menu.dropdown-menu-right
           %li.dropdown-header
-            Settings
+            = _('Settings')
             %li
               - if user.ldap_blocked?
-                %span.small Cannot unblock LDAP blocked users
+                %span.small
+                  = s_('AdminUsers|Cannot unblock LDAP blocked users')
               - elsif user.blocked?
-                = link_to 'Unblock', unblock_admin_user_path(user), method: :put
+                = link_to _('Unblock'), unblock_admin_user_path(user), method: :put
               - else
-                = link_to 'Block', block_admin_user_path(user), data: { confirm: 'USER WILL BE BLOCKED! Are you sure?' }, method: :put
+                = link_to _('Block'), block_admin_user_path(user), data: { confirm: "#{s_('AdminUsers|User will be blocked').upcase}! #{_('Are you sure')}?" }, method: :put
             - if user.access_locked?
               %li
                 = link_to _('Unlock'), unlock_admin_user_path(user), method: :put, data: { confirm: _('Are you sure?') }
@@ -48,7 +43,7 @@
                   target: '#delete-user-modal',
                   delete_user_url: admin_user_path(user),
                   block_user_url: block_admin_user_path(user),
-                  username: user.name,
+                  username: sanitize_name(user.name),
                   delete_contributions: false }, type: 'button' }
                   = s_('AdminUsers|Delete user')
 
@@ -57,6 +52,6 @@
                 target: '#delete-user-modal',
                 delete_user_url: admin_user_path(user, hard_delete: true),
                 block_user_url: block_admin_user_path(user),
-                username: user.name,
+                username: sanitize_name(user.name),
                 delete_contributions: true }, type: 'button' }
                 = s_('AdminUsers|Delete user and contributions')

app/views/admin/users/_user_detail.html.haml

+.flex-list
+  .flex-row
+    = image_tag avatar_icon_for_user(user), class: 'avatar s32 d-none d-md-flex', alt: _('Avatar for %{name}') % { name: sanitize_name(user.name) }
+    .row-main-content
+      .row-title.str-truncated-100
+        = image_tag avatar_icon_for_user(user), class: 'avatar s16 d-xs-flex d-md-none mr-1 prepend-top-2', alt: _('Avatar for %{name}') % { name: sanitize_name(user.name) }
+        = link_to user.name, admin_user_path(user), class: 'text-plain js-user-link', data: { user_id: user.id }
+
+      = render_if_exists 'admin/users/user_detail_note', user: user
+
+      - user_badges_in_admin_section(user).each do |badge|
+        - css_badge = "badge badge-#{badge[:variant]}" if badge[:variant].present?
+        %span{ class: css_badge }
+          = badge[:text]
+
+      .row-second-line.str-truncated-100
+        = mail_to user.email, user.email, class: 'text-secondary'

app/views/admin/users/index.html.haml

@@ -2,30 +2,6 @@
 - page_title "Users"
 
 %div{ class: container_class }
-  .prepend-top-default
-    = form_tag admin_users_path, method: :get do
-      - if params[:filter].present?
-        = hidden_field_tag "filter", h(params[:filter])
-      .search-holder
-        .search-field-holder
-          = search_field_tag :search_query, params[:search_query], placeholder: 'Search by name, email or username', class: 'form-control search-text-input js-search-input', spellcheck: false
-          - if @sort.present?
-            = hidden_field_tag :sort, @sort
-          = icon("search", class: "search-icon")
-          = button_tag 'Search users' if Rails.env.test?
-        .dropdown.user-sort-dropdown
-          - toggle_text = if @sort.present? then users_sort_options_hash[@sort] else sort_title_name end
-          = dropdown_toggle(toggle_text, { toggle: 'dropdown' })
-          %ul.dropdown-menu.dropdown-menu-right
-            %li.dropdown-header
-              Sort by
-            %li
-              - users_sort_options_hash.each do |value, title|
-                = link_to admin_users_path(sort: value, filter: params[:filter], search_query: params[:search_query]) do
-                  = title
-        = link_to 'Send email to users', admin_email_path, class: 'btn'
-        = link_to 'New user', new_admin_user_path, class: 'btn btn-success btn-search'
-
   .top-area.scrolling-tabs-container.inner-page-scroll-tabs
     .fade-left
       = icon('angle-left')
@@ -34,41 +10,70 @@
     %ul.nav-links.nav.nav-tabs.scrolling-tabs
       = nav_link(html_options: { class: active_when(params[:filter].nil?) }) do
         = link_to admin_users_path do
-            Active
+          = s_('AdminUsers|Active')
           %small.badge.badge-pill= limited_counter_with_delimiter(User.active)
       = nav_link(html_options: { class: active_when(params[:filter] == 'admins') }) do
         = link_to admin_users_path(filter: "admins") do
-            Admins
+          = s_('AdminUsers|Admins')
           %small.badge.badge-pill= limited_counter_with_delimiter(User.admins)
       = nav_link(html_options: { class: "#{active_when(params[:filter] == 'two_factor_enabled')} filter-two-factor-enabled" }) do
         = link_to admin_users_path(filter: 'two_factor_enabled') do
-            2FA Enabled
+          = s_('AdminUsers|2FA Enabled')
           %small.badge.badge-pill= limited_counter_with_delimiter(User.with_two_factor)
       = nav_link(html_options: { class: "#{active_when(params[:filter] == 'two_factor_disabled')} filter-two-factor-disabled" }) do
         = link_to admin_users_path(filter: 'two_factor_disabled') do
-            2FA Disabled
+          = s_('AdminUsers|2FA Disabled')
           %small.badge.badge-pill= limited_counter_with_delimiter(User.without_two_factor)
       = nav_link(html_options: { class: active_when(params[:filter] == 'external') }) do
         = link_to admin_users_path(filter: 'external') do
-            External
+          = s_('AdminUsers|External')
           %small.badge.badge-pill= limited_counter_with_delimiter(User.external)
       = nav_link(html_options: { class: active_when(params[:filter] == 'blocked') }) do
         = link_to admin_users_path(filter: "blocked") do
-            Blocked
+          = s_('AdminUsers|Blocked')
           %small.badge.badge-pill= limited_counter_with_delimiter(User.blocked)
       = nav_link(html_options: { class: active_when(params[:filter] == 'wop') }) do
         = link_to admin_users_path(filter: "wop") do
-            Without projects
+          = s_('AdminUsers|Without projects')
           %small.badge.badge-pill= limited_counter_with_delimiter(User.without_projects)
+    .nav-controls
+      = render_if_exists 'admin/users/admin_email_users'
+      = link_to s_('AdminUsers|New user'), new_admin_user_path, class: 'btn btn-success btn-search float-right'
 
-  %ul.flex-list.content-list
-    - if @users.empty?
+  .filtered-search-block.row-content-block.border-top-0
+    = form_tag admin_users_path, method: :get do
+      - if params[:filter].present?
+        = hidden_field_tag "filter", h(params[:filter])
+      .search-holder
+        .search-field-holder
+          = search_field_tag :search_query, params[:search_query], placeholder: s_('AdminUsers|Search by name, email or username'), class: 'form-control search-text-input js-search-input', spellcheck: false
+          - if @sort.present?
+            = hidden_field_tag :sort, @sort
+          = icon("search", class: "search-icon")
+          = button_tag s_('AdminUsers|Search users') if Rails.env.test?
+        .dropdown.user-sort-dropdown
+          - toggle_text = if @sort.present? then users_sort_options_hash[@sort] else sort_title_name end
+          = dropdown_toggle(toggle_text, { toggle: 'dropdown' })
+          %ul.dropdown-menu.dropdown-menu-right
+            %li.dropdown-header
+              = s_('AdminUsers|Sort by')
             %li
-        .nothing-here-block No users found.
+              - users_sort_options_hash.each do |value, title|
+                = link_to admin_users_path(sort: value, filter: params[:filter], search_query: params[:search_query]) do
+                  = title
+
+  - if @users.empty?
+    .nothing-here-block.border-top-0
+      = s_('AdminUsers|No users found')
   - else
+    .table-holder
+      .thead-white.text-nowrap.gl-responsive-table-row.table-row-header{ role: 'row' }
+        .table-section.section-40{ role: 'rowheader' }= _('Name')
+        .table-section.section-25{ role: 'rowheader' }= _('Created on')
+        .table-section.section-15{ role: 'rowheader' }= _('Last activity')
+
       = render partial: 'admin/users/user', collection: @users
 
   = paginate @users, theme: "gitlab"
 
 #delete-user-modal
-

changelogs/unreleased/43681-display-last-activity-and-created-at-datetimes-for-users-in-admin-users.yml

+---
+title: Display last activity and created at datetimes for users
+merge_request: 24181
+author:
+type: added

config/locales/de.yml

@@ -43,6 +43,7 @@ de:
       default: "%d.%m.%Y"
       long: "%e. %B %Y"
       short: "%e. %b"
+      admin: "%e %b, %Y"
     month_names:
     -
     - Januar

config/locales/en.yml

@@ -55,6 +55,7 @@ en:
       default: "%Y-%m-%d"
       long: "%B %d, %Y"
       short: "%b %d"
+      admin: "%e %b, %Y"
     month_names:
     -
     - January

config/locales/es.yml

@@ -42,6 +42,7 @@ es:
       default: "%d/%m/%Y"
       long: "%d de %B de %Y"
       short: "%d de %b"
+      admin: "%e %b, %Y"
     month_names:
     -
     - enero

ee/app/views/admin/users/_admin_email_users.html.haml

+= link_to s_('AdminUsers|Send email to users'), admin_email_path, class: 'btn'

ee/app/views/admin/users/_user_detail_note.html.haml

+- if user.note.present?
+  %span.has-tooltip.user-note{ title: user.note }
+    = icon("sticky-note-o cgrey")

locale/gitlab.pot

@@ -575,9 +575,30 @@ msgstr ""
 msgid "AdminSettings|When creating a new environment variable it will be protected by default."
 msgstr ""
 
+msgid "AdminUsers|2FA Disabled"
+msgstr ""
+
+msgid "AdminUsers|2FA Enabled"
+msgstr ""
+
+msgid "AdminUsers|Active"
+msgstr ""
+
+msgid "AdminUsers|Admin"
+msgstr ""
+
+msgid "AdminUsers|Admins"
+msgstr ""
+
 msgid "AdminUsers|Block user"
 msgstr ""
 
+msgid "AdminUsers|Blocked"
+msgstr ""
+
+msgid "AdminUsers|Cannot unblock LDAP blocked users"
+msgstr ""
+
 msgid "AdminUsers|Delete User %{username} and contributions?"
 msgstr ""
 
@@ -590,12 +611,42 @@ msgstr ""
 msgid "AdminUsers|Delete user and contributions"
 msgstr ""
 
+msgid "AdminUsers|External"
+msgstr ""
+
+msgid "AdminUsers|It's you!"
+msgstr ""
+
+msgid "AdminUsers|New user"
+msgstr ""
+
+msgid "AdminUsers|No users found"
+msgstr ""
+
+msgid "AdminUsers|Search by name, email or username"
+msgstr ""
+
+msgid "AdminUsers|Search users"
+msgstr ""
+
+msgid "AdminUsers|Send email to users"
+msgstr ""
+
+msgid "AdminUsers|Sort by"
+msgstr ""
+
 msgid "AdminUsers|To confirm, type %{projectName}"
 msgstr ""
 
 msgid "AdminUsers|To confirm, type %{username}"
 msgstr ""
 
+msgid "AdminUsers|User will be blocked"
+msgstr ""
+
+msgid "AdminUsers|Without projects"
+msgstr ""
+
 msgid "Advanced permissions, Large File Storage and Two-Factor authentication settings."
 msgstr ""
 
@@ -916,6 +967,9 @@ msgstr ""
 msgid "Archived projects"
 msgstr ""
 
+msgid "Are you sure"
+msgstr ""
+
 msgid "Are you sure you want to delete this pipeline schedule?"
 msgstr ""
 
@@ -1144,6 +1198,9 @@ msgstr ""
 msgid "Avatar for %{assigneeName}"
 msgstr ""
 
+msgid "Avatar for %{name}"
+msgstr ""
+
 msgid "Avatar will be removed. Are you sure?"
 msgstr ""
 
@@ -1327,6 +1384,9 @@ msgstr ""
 msgid "Bitbucket import"
 msgstr ""
 
+msgid "Block"
+msgstr ""
+
 msgid "Blocked"
 msgstr ""
 
@@ -5363,6 +5423,9 @@ msgstr[1] ""
 msgid "Last Pipeline"
 msgstr ""
 
+msgid "Last activity"
+msgstr ""
+
 msgid "Last commit"
 msgstr ""
 
@@ -8705,6 +8768,9 @@ msgstr ""
 msgid "SortOptions|Oldest joined"
 msgstr ""
 
+msgid "SortOptions|Oldest last activity"
+msgstr ""
+
 msgid "SortOptions|Oldest sign in"
 msgstr ""
 
@@ -8717,6 +8783,9 @@ msgstr ""
 msgid "SortOptions|Priority"
 msgstr ""
 
+msgid "SortOptions|Recent last activity"
+msgstr ""
+
 msgid "SortOptions|Recent sign in"
 msgstr ""
 
@@ -9951,6 +10020,9 @@ msgstr ""
 msgid "Unable to update this epic at this time."
 msgstr ""
 
+msgid "Unblock"
+msgstr ""
+
 msgid "Undo"
 msgstr ""
 

spec/features/admin/admin_users_spec.rb

 require 'spec_helper'
 
 describe "Admin::Users" do
-  include Spec::Support::Helpers::Features::ListRowsHelpers
+  include Spec::Support::Helpers::Features::ResponsiveTableHelpers
 
   let!(:user) do
     create(:omniauth_user, provider: 'twitter', extern_uid: '123456')
   end
 
-  let!(:current_user) { create(:admin) }
+  let!(:current_user) { create(:admin, last_activity_on: 5.days.ago) }
 
   before do
     sign_in(current_user)
@@ -25,6 +25,8 @@ describe "Admin::Users" do
     it "has users list" do
       expect(page).to have_content(current_user.email)
       expect(page).to have_content(current_user.name)
+      expect(page).to have_content(current_user.created_at.strftime("%e %b, %Y"))
+      expect(page).to have_content(current_user.last_activity_on.strftime("%e %b, %Y"))
       expect(page).to have_content(user.email)
       expect(page).to have_content(user.name)
       expect(page).to have_link('Block', href: block_admin_user_path(user))
@@ -32,10 +34,24 @@ describe "Admin::Users" do
       expect(page).to have_button('Delete user and contributions')
     end
 
+    describe "view extra user information", :js do
+      it 'does not have the user popover open' do
+        expect(page).not_to have_selector('#__BV_popover_1__')
+      end
+
+      it 'shows the user popover on hover' do
+        first_user_link = page.first('.js-user-link')
+
+        first_user_link.hover
+
+        expect(page).to have_selector('#__BV_popover_1__')
+      end
+    end
+
     describe 'search and sort' do
       before do
-        create(:user, name: 'Foo Bar')
-        create(:user, name: 'Foo Baz')
+        create(:user, name: 'Foo Bar', last_activity_on: 3.days.ago)
+        create(:user, name: 'Foo Baz', last_activity_on: 2.days.ago)
         create(:user, name: 'Dmitriy')
       end
 
@@ -75,6 +91,24 @@ describe "Admin::Users" do
         expect(first_row.text).to include('Foo Bar')
         expect(second_row.text).to include('Foo Baz')
       end
+
+      it 'sorts users by recent last activity' do
+        visit admin_users_path(search_query: 'Foo')
+
+        sort_by('Recent last activity')
+
+        expect(first_row.text).to include('Foo Baz')
+        expect(second_row.text).to include('Foo Bar')
+      end
+
+      it 'sorts users by oldest last activity' do
+        visit admin_users_path(search_query: 'Foo')
+
+        sort_by('Oldest last activity')
+
+        expect(first_row.text).to include('Foo Bar')
+        expect(second_row.text).to include('Foo Baz')
+      end
     end
 
     describe 'Two-factor Authentication filters' do

spec/helpers/users_helper_spec.rb

@@ -100,4 +100,72 @@ describe UsersHelper do
       end
     end
   end
+
+  describe '#user_badges_in_admin_section' do
+    before do
+      allow(helper).to receive(:current_user).and_return(user)
+    end
+
+    context 'with a blocked user' do
+      it "returns the blocked badge" do
+        blocked_user = create(:user, state: 'blocked')
+
+        badges = helper.user_badges_in_admin_section(blocked_user)
+
+        expect(badges).to eq([text: "Blocked", variant: "danger"])
+      end
+    end
+
+    context 'with an admin user' do
+      it "returns the admin badge" do
+        admin_user = create(:admin)
+
+        badges = helper.user_badges_in_admin_section(admin_user)
+
+        expect(badges).to eq([text: "Admin", variant: "success"])
+      end
+    end
+
+    context 'with an external user' do
+      it 'returns the external badge' do
+        external_user = create(:user, external: true)
+
+        badges = helper.user_badges_in_admin_section(external_user)
+
+        expect(badges).to eq([text: "External", variant: "secondary"])
+      end
+    end
+
+    context 'with the current user' do
+      it 'returns the "It\'s You" badge' do
+        badges = helper.user_badges_in_admin_section(user)
+
+        expect(badges).to eq([text: "It's you!", variant: nil])
+      end
+    end
+
+    context 'with an external blocked admin' do
+      it 'returns the blocked, admin and external badges' do
+        user = create(:admin, state: 'blocked', external: true)
+
+        badges = helper.user_badges_in_admin_section(user)
+
+        expect(badges).to eq([
+          { text: "Blocked", variant: "danger" },
+          { text: "Admin", variant: "success" },
+          { text: "External", variant: "secondary" }
+        ])
+      end
+    end
+
+    context 'get badges for normal user' do
+      it 'returns no badges' do
+        user = create(:user)
+
+        badges = helper.user_badges_in_admin_section(user)
+
+        expect(badges).to be_empty
+      end
+    end
+  end
 end

spec/support/helpers/features/responsive_table_helpers.rb

+# frozen_string_literal: true
+# These helpers allow you to access rows in a responsive table
+#
+# Usage:
+#   describe "..." do
+#   include Spec::Support::Helpers::Features::ResponsiveTableHelpers
+#     ...
+#
+#     expect(first_row.text).to include("John Doe")
+#     expect(second_row.text).to include("John Smith")
+#
+# Note:
+#   index starts at 1 as index 0 is expected to be the table header
+#
+#
+module Spec
+  module Support
+    module Helpers
+      module Features
+        module ResponsiveTableHelpers
+          def first_row
+            page.all('.gl-responsive-table-row')[1]
+          end
+
+          def second_row
+            page.all('.gl-responsive-table-row')[2]
+          end
+        end
+      end
+    end
+  end
+end