UX: admins users page follows admin ux guideline (#29873)

Conversion of `/admin/users` page to follow admin UX guidelines. In addition, add the username to the title on the user admin page.
2024-12-02 10:11:23 +11:00 · 2024-12-02 10:11:23 +11:00 · 6d4c6ee154
parent 976be2abcd
commit 6d4c6ee154
9 changed files with 225 additions and 97 deletions
--- a/app/assets/javascripts/admin/addon/routes/admin-user-index.js
+++ b/app/assets/javascripts/admin/addon/routes/admin-user-index.js
@ -6,6 +6,10 @@ export default class AdminUserIndexRoute extends DiscourseRoute {
    return this.modelFor("adminUser");
  }

+  titleToken() {
+    return this.currentModel.username;
+  }
+
  afterModel(model) {
    if (this.currentUser.admin) {
      return Group.findAll().then((groups) => {
--- a/app/assets/javascripts/admin/addon/templates/users-list-show.hbs
+++ b/app/assets/javascripts/admin/addon/templates/users-list-show.hbs
@ -1,26 +1,27 @@
-<PluginOutlet @name="admin-users-list-show-before" />
-
-<div class="admin-title">
-  <h2>{{this.title}}</h2>
+<AdminPageSubheader @titleLabelTranslated={{this.title}}>
+  <:actions as |actions|>
    {{#if this.canCheckEmails}}
      {{#if this.showEmails}}
-      <DButton
+        <actions.Default
          @action={{this.toggleEmailVisibility}}
          @label="admin.users.hide_emails"
-        class="hide-emails btn-default"
+          class="admin-users__subheader-hide-emails"
        />
      {{else}}
-      <DButton
+        <actions.Default
          @action={{this.toggleEmailVisibility}}
          @label="admin.users.show_emails"
-        class="show-emails btn-default"
+          class="admin-users__subheader-show-emails"
        />
      {{/if}}
    {{/if}}
-</div>
+  </:actions>
+</AdminPageSubheader>
+
+<PluginOutlet @name="admin-users-list-show-before" />

 <div class="admin-users-list__controls">
-  <div class="username">
+  <div class="admin-users-list__search">
    <input
      type="text"
      dir="auto"
@ -64,7 +65,7 @@
 >
  {{#if this.users}}
    <ResponsiveTable
-      @className="users-list"
+      @className={{concat-class "users-list" this.query}}
      @aria-label={{this.title}}
      @style={{html-safe
        (concat
--- a/app/assets/javascripts/admin/addon/templates/users-list.hbs
+++ b/app/assets/javascripts/admin/addon/templates/users-list.hbs
@ -1,70 +1,82 @@
-<div class="admin-controls">
-  <nav>
-    <ul class="nav nav-pills">
+<div class="admin-users admin-config-page">
+  <AdminPageHeader
+    @titleLabel="admin.users.title"
+    @descriptionLabel="admin.users.description"
+    @learnMoreUrl="https://meta.discourse.org/t/accessing-a-user-s-admin-page/311859"
+  >
+    <:breadcrumbs>
+      <DBreadcrumbsItem
+        @path="/admin/users/list"
+        @label={{i18n "admin.permalink.title"}}
+      />
+    </:breadcrumbs>
+    <:actions as |actions|>
+      {{#if this.currentUser.can_invite_to_forum}}
+        <actions.Primary
+          @action={{route-action "sendInvites"}}
+          @title="admin.invite.button_title"
+          @label="admin.invite.button_text"
+          class="admin-users__header-send-invites"
+        />
+      {{/if}}
+
+      {{#if this.currentUser.admin}}
+        <actions.Primary
+          @action={{route-action "exportUsers"}}
+          @title="admin.export_csv.button_title.user"
+          @label="admin.export_csv.button_text"
+          class="admin-users__header-export-users"
+        />
+      {{/if}}
+    </:actions>
+    <:tabs>
      <NavItem
        @route="adminUsersList.show"
        @routeParam="active"
        @label="admin.users.nav.active"
-        class="active-users"
+        class="admin-users-tabs__active"
      />
      <NavItem
        @route="adminUsersList.show"
        @routeParam="new"
        @label="admin.users.nav.new"
-        class="new-users"
+        class="admin-users-tabs__new"
      />
      <NavItem
        @route="adminUsersList.show"
        @routeParam="staff"
        @label="admin.users.nav.staff"
-        class="staff-users"
+        class="admin-users-tabs__staff"
      />
      <NavItem
        @route="adminUsersList.show"
        @routeParam="suspended"
        @label="admin.users.nav.suspended"
-        class="suspended-users"
+        class="admin-users-tabs__suspended"
      />
      <NavItem
        @route="adminUsersList.show"
        @routeParam="silenced"
        @label="admin.users.nav.silenced"
-        class="silenced-users"
+        class="admin-users-tabs__silenced"
      />
      <NavItem
        @route="adminUsersList.show"
        @routeParam="staged"
        @label="admin.users.nav.staged"
-        class="staged-users"
+        class="admin-users-tabs__staged"
      />
-      <NavItem @route="groups" @label="groups.index.title" class="groups" />
-      <PluginOutlet @name="admin-users-list-nav-after" />
-
-      <li class="admin-actions">
-        {{#if this.currentUser.can_invite_to_forum}}
-          <DButton
-            @action={{route-action "sendInvites"}}
-            @title="admin.invite.button_title"
-            @icon="user-plus"
-            @label="admin.invite.button_text"
-            class="btn-flat"
+      <NavItem
+        @route="groups"
+        @label="groups.index.title"
+        class="admin-users-tabs__groups"
      />
-        {{/if}}
-
-        {{#if this.currentUser.admin}}
-          <DButton
-            @action={{route-action "exportUsers"}}
-            @title="admin.export_csv.button_title.user"
-            @icon="download"
-            @label="admin.export_csv.button_text"
-            class="btn-flat"
-          />
-        {{/if}}
-      </li>
-    </ul>
-  </nav>
+    </:tabs>
+  </AdminPageHeader>
+  <div class="admin-container admin-config-page__main-area">
+  </div>
 </div>

-<div class="admin-container">
+<div class="admin-container admin-config-page__main-area">
  {{outlet}}
 </div>
--- a/app/assets/javascripts/discourse/tests/acceptance/admin-users-list-test.js
+++ b/app/assets/javascripts/discourse/tests/acceptance/admin-users-list-test.js
@ -18,7 +18,7 @@ acceptance("Admin - Users List", function (needs) {
  test("searching users with no matches", async function (assert) {
    await visit("/admin/users/list/active");

-    await fillIn(".admin-users-list__controls .username input", "doesntexist");
+    await fillIn(".admin-users-list__search input", "doesntexist");

    assert.dom(".users-list-container").hasText(i18n("search.no_results"));
  });
@ -50,13 +50,13 @@ acceptance("Admin - Users List", function (needs) {

    assert.dom(".users-list .user").exists();

-    await click(".show-emails");
+    await click(".admin-users__subheader-show-emails");

    assert
      .dom(".users-list .user:nth-child(1) .email")
      .hasText("<small>eviltrout@example.com</small>", "shows the emails");

-    await click(".hide-emails");
+    await click(".admin-users__subheader-hide-emails");

    assert
      .dom(".users-list .user:nth-child(1) .email .directory-table__value")
@ -71,28 +71,28 @@ acceptance("Admin - Users List", function (needs) {

    await visit("/admin/users/list/active");

-    assert.dom(".admin-title h2").hasText(activeTitle);
+    assert.dom(".admin-page-subheader__title").hasText(activeTitle);
    assert
      .dom(".users-list .user:nth-child(1) .username")
      .includesText(activeUser);

    await click('a[href="/admin/users/list/new"]');

-    assert.dom(".admin-title h2").hasText(suspectTitle);
+    assert.dom(".admin-page-subheader__title").hasText(suspectTitle);
    assert
      .dom(".users-list .user:nth-child(1) .username")
      .includesText(suspectUser);

    await click(".users-list .sortable:nth-child(4)");

-    assert.dom(".admin-title h2").hasText(suspectTitle);
+    assert.dom(".admin-page-subheader__title").hasText(suspectTitle);
    assert
      .dom(".users-list .user:nth-child(1) .username")
      .includesText(suspectUser);

    await click('a[href="/admin/users/list/active"]');

-    assert.dom(".admin-title h2").hasText(activeTitle);
+    assert.dom(".admin-page-subheader__title").hasText(activeTitle);
    assert
      .dom(".users-list .user:nth-child(1) .username")
      .includesText(activeUser);
--- a/app/assets/stylesheets/common/admin/admin_base.scss
+++ b/app/assets/stylesheets/common/admin/admin_base.scss
@ -423,6 +423,20 @@ $mobile-breakpoint: 700px;
    color: var(--primary-medium);
  }
 }
+.admin-users-list {
+  &__search {
+    @media screen and (max-width: 500px) {
+      width: 100%;
+    }
+    input {
+      min-width: 15em;
+      @media screen and (max-width: 500px) {
+        box-sizing: border-box;
+        width: 100%;
+      }
+    }
+  }
+}

 .ip-lookup {
  position: relative;
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@ -6642,6 +6642,7 @@ en:

      users:
        title: "Users"
+        description: "View and manage users."
        create: "Add Admin User"
        last_emailed: "Last Emailed"
        not_found: "Sorry, that username doesn't exist in our system."
--- a/spec/system/admin_user_spec.rb
+++ b/spec/system/admin_user_spec.rb
@ -44,6 +44,11 @@ describe "Admin User Page", type: :system do
      expect(admin_user_page).to have_silence_button
    end

+    it "displays username in the title" do
+      expect(page).to have_css(".display-row.username")
+      expect(page.title).to eq("#{user.username} - Admin - Discourse")
+    end
+
    describe "the suspend user modal" do
      it "displays the list of users who share the same IP but are not mods or admins" do
        admin_user_page.click_suspend_button
--- a/spec/system/admin_users_list_spec.rb
+++ b/spec/system/admin_users_list_spec.rb
@ -3,7 +3,9 @@
 describe "Admin Users Page", type: :system do
  fab!(:current_user) { Fabricate(:admin) }
  fab!(:another_admin) { Fabricate(:admin) }
-  fab!(:users) { Fabricate.times(3, :user) }
+  fab!(:user_1) { Fabricate(:user) }
+  fab!(:user_2) { Fabricate(:user) }
+  fab!(:user_3) { Fabricate(:user) }

  let(:admin_users_page) { PageObjects::Pages::AdminUsers.new }

@ -19,7 +21,7 @@ describe "Admin Users Page", type: :system do

      expect(admin_users_page.user_row(current_user.id).bulk_select_checkbox.disabled?).to eq(true)
      expect(admin_users_page.user_row(another_admin.id).bulk_select_checkbox.disabled?).to eq(true)
-      expect(admin_users_page.user_row(users[0].id).bulk_select_checkbox.disabled?).to eq(false)
+      expect(admin_users_page.user_row(user_1.id).bulk_select_checkbox.disabled?).to eq(false)

      admin_users_page.user_row(another_admin.id).bulk_select_checkbox.hover
      expect(PageObjects::Components::Tooltips.new("bulk-delete-unavailable-reason")).to be_present(
@ -30,25 +32,25 @@ describe "Admin Users Page", type: :system do
    it "has a button that toggles the bulk select checkboxes" do
      admin_users_page.visit

-      expect(admin_users_page).to have_users(users.map(&:id))
+      expect(admin_users_page).to have_users([user_1.id, user_2.id, user_3.id])

-      expect(admin_users_page.user_row(users[0].id)).to have_no_bulk_select_checkbox
-      expect(admin_users_page.user_row(users[1].id)).to have_no_bulk_select_checkbox
-      expect(admin_users_page.user_row(users[2].id)).to have_no_bulk_select_checkbox
+      expect(admin_users_page.user_row(user_1.id)).to have_no_bulk_select_checkbox
+      expect(admin_users_page.user_row(user_2.id)).to have_no_bulk_select_checkbox
+      expect(admin_users_page.user_row(user_3.id)).to have_no_bulk_select_checkbox

      admin_users_page.bulk_select_button.click

-      expect(admin_users_page.user_row(users[0].id)).to have_bulk_select_checkbox
-      expect(admin_users_page.user_row(users[1].id)).to have_bulk_select_checkbox
-      expect(admin_users_page.user_row(users[2].id)).to have_bulk_select_checkbox
+      expect(admin_users_page.user_row(user_1.id)).to have_bulk_select_checkbox
+      expect(admin_users_page.user_row(user_2.id)).to have_bulk_select_checkbox
+      expect(admin_users_page.user_row(user_3.id)).to have_bulk_select_checkbox

      expect(admin_users_page).to have_no_bulk_actions_dropdown

-      admin_users_page.user_row(users[0].id).bulk_select_checkbox.click
+      admin_users_page.user_row(user_1.id).bulk_select_checkbox.click

      expect(admin_users_page).to have_bulk_actions_dropdown

-      admin_users_page.user_row(users[1].id).bulk_select_checkbox.click
+      admin_users_page.user_row(user_2.id).bulk_select_checkbox.click
      admin_users_page.bulk_actions_dropdown.expand
      admin_users_page.bulk_actions_dropdown.option(".bulk-delete").click

@ -64,38 +66,37 @@ describe "Admin Users Page", type: :system do
      confirmation_modal.confirm_button.click

      expect(confirmation_modal).to have_successful_log_entry_for_user(
-        user: users[0],
+        user: user_1,
        position: 1,
        total: 2,
      )
      expect(confirmation_modal).to have_successful_log_entry_for_user(
-        user: users[1],
+        user: user_2,
        position: 2,
        total: 2,
      )
      expect(confirmation_modal).to have_no_error_log_entries

      confirmation_modal.close
-      deleted_ids = users[0..1].map(&:id)
-      expect(admin_users_page).to have_no_users(deleted_ids)
-      expect(User.where(id: deleted_ids).count).to eq(0)
+      expect(admin_users_page).to have_no_users([user_1.id, user_2.id])
+      expect(User.where(id: [user_1.id, user_2.id]).count).to eq(0)
    end

    it "remembers selected users when the user list refreshes due to search" do
      admin_users_page.visit
      admin_users_page.bulk_select_button.click
-      admin_users_page.search_input.fill_in(with: users[0].username)
-      admin_users_page.user_row(users[0].id).bulk_select_checkbox.click
+      admin_users_page.search_input.fill_in(with: user_1.username)
+      admin_users_page.user_row(user_1.id).bulk_select_checkbox.click

-      admin_users_page.search_input.fill_in(with: users[1].username)
-      admin_users_page.user_row(users[1].id).bulk_select_checkbox.click
+      admin_users_page.search_input.fill_in(with: user_2.username)
+      admin_users_page.user_row(user_2.id).bulk_select_checkbox.click

      admin_users_page.search_input.fill_in(with: "")

-      expect(admin_users_page).to have_users(users.map(&:id))
-      expect(admin_users_page.user_row(users[0].id).bulk_select_checkbox).to be_checked
-      expect(admin_users_page.user_row(users[1].id).bulk_select_checkbox).to be_checked
-      expect(admin_users_page.user_row(users[2].id).bulk_select_checkbox).not_to be_checked
+      expect(admin_users_page).to have_users([user_1.id, user_2.id, user_3.id])
+      expect(admin_users_page.user_row(user_1.id).bulk_select_checkbox).to be_checked
+      expect(admin_users_page.user_row(user_2.id).bulk_select_checkbox).to be_checked
+      expect(admin_users_page.user_row(user_3.id).bulk_select_checkbox).not_to be_checked

      admin_users_page.bulk_actions_dropdown.expand
      admin_users_page.bulk_actions_dropdown.option(".bulk-delete").click
@ -104,39 +105,91 @@ describe "Admin Users Page", type: :system do
      confirmation_modal.fill_in_confirmation_phase(user_count: 2)
      confirmation_modal.confirm_button.click
      expect(confirmation_modal).to have_successful_log_entry_for_user(
-        user: users[0],
+        user: user_1,
        position: 1,
        total: 2,
      )
      expect(confirmation_modal).to have_successful_log_entry_for_user(
-        user: users[1],
+        user: user_2,
        position: 2,
        total: 2,
      )
      confirmation_modal.close

-      deleted_ids = users[0..1].map(&:id)
-      expect(admin_users_page).to have_no_users(deleted_ids)
-      expect(User.where(id: deleted_ids).count).to eq(0)
+      expect(admin_users_page).to have_no_users([user_1.id, user_2.id])
+      expect(User.where(id: [user_1.id, user_2.id]).count).to eq(0)
    end

    it "displays an error message if bulk delete fails" do
      admin_users_page.visit
      admin_users_page.bulk_select_button.click

-      admin_users_page.user_row(users[0].id).bulk_select_checkbox.click
+      admin_users_page.user_row(user_1.id).bulk_select_checkbox.click
      admin_users_page.bulk_actions_dropdown.expand
      admin_users_page.bulk_actions_dropdown.option(".bulk-delete").click
      confirmation_modal.fill_in_confirmation_phase(user_count: 1)

-      users[0].update!(admin: true)
+      user_1.update!(admin: true)

      confirmation_modal.confirm_button.click
      expect(confirmation_modal).to have_error_log_entry(
        I18n.t("js.generic_error_with_reason", error: I18n.t("user.cannot_bulk_delete")),
      )
      confirmation_modal.close
-      expect(admin_users_page).to have_users([users[0].id])
+      expect(admin_users_page).to have_users([user_1.id])
+    end
+  end
+
+  context "when visiting an admin's page" do
+    it "shows list of active users" do
+      admin_users_page.visit
+      expect(admin_users_page).to have_active_tab("active")
+      expect(page).to have_css(".directory-table__cell.username")
+      expect(admin_users_page).to have_users(
+        [current_user.id, another_admin.id, user_1.id, user_2.id, user_3.id],
+      )
+    end
+
+    it "shows list of suspended users" do
+      admin_users_page.visit
+      admin_users_page.click_tab("suspended")
+      expect(admin_users_page).to have_active_tab("suspended")
+      expect(admin_users_page).to have_none_users
+    end
+
+    it "shows list of silenced users" do
+      admin_users_page.visit
+      user_1.update!(silenced_till: 1.day.from_now)
+      admin_users_page.click_tab("silenced")
+      expect(admin_users_page).to have_active_tab("silenced")
+      expect(page).to have_css(".users-list.silenced")
+      expect(admin_users_page).to have_users([user_1.id])
+    end
+
+    it "shows emails" do
+      admin_users_page.visit
+      expect(admin_users_page).to have_no_emails
+      admin_users_page.click_show_emails
+      expect(admin_users_page).to have_emails
+    end
+
+    it "redirects to groups page" do
+      admin_users_page.visit
+      admin_users_page.click_tab("groups")
+      expect(page).to have_current_path("/g")
+    end
+
+    it "redirect to invites page" do
+      admin_users_page.visit
+      admin_users_page.click_send_invites
+      expect(page).to have_current_path("/u/#{current_user.username}/invited/pending")
+    end
+
+    it "allows to export users" do
+      admin_users_page.visit
+      admin_users_page.click_export
+      expect(page).to have_css(".dialog-body")
+      expect(page).to have_content(I18n.t("admin_js.admin.export_csv.success"))
    end
  end
 end
--- a/spec/system/page_objects/pages/admin_users.rb
+++ b/spec/system/page_objects/pages/admin_users.rb
@ -32,7 +32,7 @@ module PageObjects
      end

      def search_input
-        find(".admin-users-list__controls .username input")
+        find(".admin-users-list__search input")
      end

      def user_row(id)
@ -62,6 +62,44 @@ module PageObjects
      def has_no_bulk_actions_dropdown?
        has_no_css?(".bulk-select-admin-users-dropdown-trigger")
      end
+
+      def has_usernames?(usernames)
+        expect(all(".directory-table__cell.username").map(&:text)).to eq(usernames)
+      end
+
+      def has_none_users?
+        has_content?(I18n.t("js.search.no_results"))
+      end
+
+      def click_tab(tab)
+        has_css?(".admin-users-tabs__#{tab}")
+        find(".admin-users-tabs__#{tab}").click
+      end
+
+      def has_active_tab?(tab)
+        has_css?(".admin-users-tabs__#{tab} .active")
+        has_no_css?(".loading-container .visible")
+      end
+
+      def has_no_emails?
+        has_no_css?(".directory-table__column-header--email")
+      end
+
+      def has_emails?
+        has_css?(".directory-table__column-header--email")
+      end
+
+      def click_show_emails
+        find(".admin-users__subheader-show-emails").click
+      end
+
+      def click_send_invites
+        find(".admin-users__header-send-invites").click
+      end
+
+      def click_export
+        find(".admin-users__header-export-users").click
+      end
    end
  end
 end