DEV: Standardize table sorting verbiage (#9757)

* DEV: Standardize table sorting verbiage

This commit creates a common component that tables can use to make their
headers sortable. This commit also standardizes on using `desc` as the
default and passing in the `asc=true` flag to adjust the sorting
direction.

* Add deprecation warnings

Adds deprecation warnings if using previous params and maintains
backwards compatibility. Set the default sort value for group members to
be asc.

* switch group requests to use common table-header-toggle

* update fixture

app/assets/javascripts/admin/components/admin-directory-toggle.js

@@ -1,30 +0,0 @@
-import Component from "@ember/component";
-import { iconHTML } from "discourse-common/lib/icon-library";
-
-export default Component.extend({
-  tagName: "th",
-  classNames: ["sortable"],
-  chevronIcon: null,
-  toggleProperties() {
-    if (this.order === this.field) {
-      this.set("ascending", this.ascending ? null : true);
-    } else {
-      this.setProperties({ order: this.field, ascending: null });
-    }
-  },
-  toggleChevron() {
-    if (this.order === this.field) {
-      let chevron = iconHTML(this.ascending ? "chevron-up" : "chevron-down");
-      this.set("chevronIcon", `${chevron}`.htmlSafe());
-    } else {
-      this.set("chevronIcon", null);
-    }
-  },
-  click() {
-    this.toggleProperties();
-  },
-  didReceiveAttrs() {
-    this._super(...arguments);
-    this.toggleChevron();
-  }
-});

app/assets/javascripts/admin/controllers/admin-users-list-show.js

@@ -11,7 +11,7 @@ export default Controller.extend(CanCheckEmails, {
   model: null,
   query: null,
   order: null,
-  ascending: null,
+  asc: null,
   showEmails: false,
   refreshing: false,
   listFilter: null,
@@ -54,7 +54,7 @@ export default Controller.extend(CanCheckEmails, {
       filter: this.listFilter,
       show_emails: this.showEmails,
       order: this.order,
-      ascending: this.ascending,
+      asc: this.asc,
       page: this._page
     })
       .then(result => {

app/assets/javascripts/admin/routes/admin-users-list-show.js

@@ -3,7 +3,7 @@ import DiscourseRoute from "discourse/routes/discourse";
 export default DiscourseRoute.extend({
   queryParams: {
     order: { refreshModel: true },
-    ascending: { refreshModel: true }
+    asc: { refreshModel: true }
   },
 
   // TODO: this has been introduced to fix a bug in admin-users-list-show
@@ -18,7 +18,7 @@ export default DiscourseRoute.extend({
       if (controller) {
         controller.setProperties({
           order: transition.to.queryParams.order,
-          ascending: transition.to.queryParams.ascending,
+          asc: transition.to.queryParams.asc,
           query: params.filter,
           refreshing: false
         });

app/assets/javascripts/admin/templates/components/admin-directory-toggle.hbs

@@ -1 +0,0 @@
-<span class="header-contents">{{i18n this.i18nKey}}{{chevronIcon}}</span>

app/assets/javascripts/admin/templates/users-list-show.hbs

@@ -23,14 +23,14 @@
   {{#if model}}
     <table class="table users-list grid">
       <thead>
-        {{admin-directory-toggle field="username" i18nKey="username" order=order ascending=ascending}}
-        {{admin-directory-toggle field="email" i18nKey="email" order=order ascending=ascending}}
-        {{admin-directory-toggle field="last_emailed" i18nKey="admin.users.last_emailed" order=order ascending=ascending}}
-        {{admin-directory-toggle field="seen" i18nKey="last_seen" order=order ascending=ascending}}
-        {{admin-directory-toggle field="topics_viewed" i18nKey="admin.user.topics_entered" order=order ascending=ascending}}
-        {{admin-directory-toggle field="posts_read" i18nKey="admin.user.posts_read_count" order=order ascending=ascending}}
-        {{admin-directory-toggle field="read_time" i18nKey="admin.user.time_read" order=order ascending=ascending}}
-        {{admin-directory-toggle field="created" i18nKey="created" order=order ascending=ascending}}
+        {{table-header-toggle field="username" labelKey="username" order=order asc=asc}}
+        {{table-header-toggle field="email" labelKey="email" order=order asc=asc}}
+        {{table-header-toggle field="last_emailed" labelKey="admin.users.last_emailed" order=order asc=asc}}
+        {{table-header-toggle field="seen" labelKey="last_seen" order=order asc=asc}}
+        {{table-header-toggle field="topics_viewed" labelKey="admin.user.topics_entered" order=order asc=asc}}
+        {{table-header-toggle field="posts_read" labelKey="admin.user.posts_read_count" order=order asc=asc}}
+        {{table-header-toggle field="read_time" labelKey="admin.user.time_read" order=order asc=asc}}
+        {{table-header-toggle field="created" labelKey="created" order=order asc=asc}}
         {{#if siteSettings.must_approve_users}}
           <th>{{i18n "admin.users.approved"}}</th>
         {{/if}}

app/assets/javascripts/discourse/app/components/group-index-toggle.js

@@ -1,30 +0,0 @@
-import Component from "@ember/component";
-import { iconHTML } from "discourse-common/lib/icon-library";
-
-export default Component.extend({
-  tagName: "th",
-  classNames: ["sortable"],
-  chevronIcon: null,
-  toggleProperties() {
-    if (this.order === this.field) {
-      this.set("desc", this.desc ? null : true);
-    } else {
-      this.setProperties({ order: this.field, desc: null });
-    }
-  },
-  toggleChevron() {
-    if (this.order === this.field) {
-      let chevron = iconHTML(this.desc ? "chevron-down" : "chevron-up");
-      this.set("chevronIcon", `${chevron}`.htmlSafe());
-    } else {
-      this.set("chevronIcon", null);
-    }
-  },
-  click() {
-    this.toggleProperties();
-  },
-  didReceiveAttrs() {
-    this._super(...arguments);
-    this.toggleChevron();
-  }
-});

app/assets/javascripts/discourse/app/controllers/group-index.js

@@ -8,10 +8,10 @@ import { action } from "@ember/object";
 export default Controller.extend({
   application: controller(),
 
-  queryParams: ["order", "desc", "filter"],
+  queryParams: ["order", "asc", "filter"],
 
   order: "",
-  desc: null,
+  asc: true,
   filter: null,
   filterInput: null,
 
@@ -24,7 +24,7 @@ export default Controller.extend({
     this.set("filter", this.filterInput);
   }, 500),
 
-  @observes("order", "desc", "filter")
+  @observes("order", "asc", "filter")
   _filtersChanged() {
     this.findMembers(true);
   },
@@ -49,9 +49,9 @@ export default Controller.extend({
     });
   },
 
-  @discourseComputed("order", "desc", "filter")
-  memberParams(order, desc, filter) {
-    return { order, desc, filter };
+  @discourseComputed("order", "asc", "filter")
+  memberParams(order, asc, filter) {
+    return { order, asc, filter };
   },
 
   hasMembers: gt("model.members.length", 0),

app/assets/javascripts/discourse/app/templates/components/group-index-toggle.hbs

@@ -1 +0,0 @@
-<span class="header-contents">{{i18n this.i18nKey}}{{chevronIcon}}</span>

app/assets/javascripts/discourse/app/templates/group-index.hbs

@@ -29,11 +29,11 @@
     {{#load-more selector=".group-members tr" action=(action "loadMore")}}
       <table class="group-members">
         <thead>
-          {{group-index-toggle order=order desc=desc field="username_lower" i18nKey="username"}}
+          {{table-header-toggle order=order asc=asc field="username_lower" labelKey="username"}}
           <th class="group-owner">{{i18n "groups.members.owner"}}</th>
-          {{group-index-toggle order=order desc=desc field="added_at" i18nKey="groups.member_added"}}
-          {{group-index-toggle order=order desc=desc field="last_posted_at" i18nKey="last_post"}}
-          {{group-index-toggle order=order desc=desc field="last_seen_at" i18nKey="last_seen"}}
+          {{table-header-toggle order=order asc=asc field="added_at" labelKey="groups.member_added"}}
+          {{table-header-toggle order=order asc=asc field="last_posted_at" labelKey="last_post"}}
+          {{table-header-toggle order=order asc=asc field="last_seen_at" labelKey="last_seen"}}
           <th></th>
         </thead>
 

app/assets/javascripts/discourse/app/templates/group-requests.hbs

@@ -12,8 +12,8 @@
     {{#load-more selector=".group-members tr" action=(action "loadMore")}}
       <table class="group-members">
         <thead>
-          {{group-index-toggle order=order desc=desc field="username_lower" i18nKey="username"}}
-          {{group-index-toggle order=order desc=desc field="requested_at" i18nKey="groups.member_requested"}}
+          {{table-header-toggle order=order asc=asc field="username_lower" labelKey="username"}}
+          {{table-header-toggle order=order asc=asc field="requested_at" labelKey="groups.member_requested"}}
           <th>{{i18n "groups.requests.reason"}}</th>
           <th></th>
           <th></th>

app/assets/javascripts/discourse/app/templates/users.hbs

@@ -22,13 +22,13 @@
             <table>
               <thead>
                 <th>{{i18n "directory.total_rows" count=model.totalRows}}</th>
-                {{directory-toggle field="likes_received" order=order asc=asc icon="heart"}}
-                {{directory-toggle field="likes_given" order=order asc=asc icon="heart"}}
-                {{directory-toggle field="topic_count" order=order asc=asc}}
-                {{directory-toggle field="post_count" order=order asc=asc}}
-                {{directory-toggle field="topics_entered" order=order asc=asc}}
-                {{directory-toggle field="posts_read" order=order asc=asc}}
-                {{directory-toggle field="days_visited" order=order asc=asc}}
+                {{table-header-toggle field="likes_received" order=order asc=asc icon="heart"}}
+                {{table-header-toggle field="likes_given" order=order asc=asc icon="heart"}}
+                {{table-header-toggle field="topic_count" order=order asc=asc}}
+                {{table-header-toggle field="post_count" order=order asc=asc}}
+                {{table-header-toggle field="topics_entered" order=order asc=asc}}
+                {{table-header-toggle field="posts_read" order=order asc=asc}}
+                {{table-header-toggle field="days_visited" order=order asc=asc}}
                 {{#if showTimeRead}}
                   <th>{{i18n "directory.time_read"}}</th>
                 {{/if}}

app/controllers/groups_controller.rb

@@ -209,7 +209,11 @@ class GroupsController < ApplicationController
     raise Discourse::InvalidParameters.new(:limit) if limit < 0 || limit > 1000
     raise Discourse::InvalidParameters.new(:offset) if offset < 0
 
-    dir = (params[:desc] && params[:desc].present?) ? 'DESC' : 'ASC'
+    dir = (params[:asc] && params[:asc].present?) ? 'ASC' : 'DESC'
+    if params[:desc]
+      Discourse.deprecate(":desc is deprecated please use :asc instead", output_in_test: true)
+      dir = (params[:desc] && params[:desc].present?) ? 'DESC' : 'ASC'
+    end
     order = ""
 
     if params[:requesters]

lib/admin_user_index_query.rb

@@ -37,7 +37,8 @@ class AdminUserIndexQuery
   end
 
   def custom_direction
-    asc = params[:ascending]
+    Discourse.deprecate(":ascending is deprecated please use :asc instead", output_in_test: true) if params[:ascending]
+    asc = params[:asc] || params[:ascending]
     asc.present? && asc ? "ASC" : "DESC"
   end
 

spec/components/admin_user_index_query_spec.rb

@@ -29,7 +29,7 @@ describe AdminUserIndexQuery do
     end
 
     it "allows custom ordering asc" do
-      query = ::AdminUserIndexQuery.new(order: "trust_level", ascending: true)
+      query = ::AdminUserIndexQuery.new(order: "trust_level", asc: true)
       expect(query.find_users_query.to_sql).to match("trust_level ASC")
     end
 
@@ -39,7 +39,7 @@ describe AdminUserIndexQuery do
     end
 
     it "allows custom ordering and direction for stats" do
-      query = ::AdminUserIndexQuery.new(order: "topics_viewed", ascending: true)
+      query = ::AdminUserIndexQuery.new(order: "topics_viewed", asc: true)
       expect(query.find_users_query.to_sql).to match("topics_entered ASC")
     end
   end
@@ -118,7 +118,7 @@ describe AdminUserIndexQuery do
     end
 
     it "shows nil values first with asc" do
-      users = ::AdminUserIndexQuery.new(order: "last_emailed", ascending: true).find_users
+      users = ::AdminUserIndexQuery.new(order: "last_emailed", asc: true).find_users
 
       expect(users.where('users.id > -2').count).to eq(2)
       expect(users.where('users.id > -2').order('users.id asc').first.username).to eq("system")

spec/requests/groups_controller_spec.rb

@@ -489,7 +489,7 @@ describe GroupsController do
       4.times { group.add(Fabricate(:user)) }
       usernames = group.users.map { |m| m.username }.sort
 
-      get "/groups/#{group.name}/members.json", params: { limit: 3 }
+      get "/groups/#{group.name}/members.json", params: { limit: 3, asc: true }
 
       expect(response.status).to eq(200)
 
@@ -497,7 +497,7 @@ describe GroupsController do
 
       expect(members.map { |m| m['username'] }).to eq(usernames[0..2])
 
-      get "/groups/#{group.name}/members.json", params: { limit: 3, offset: 3 }
+      get "/groups/#{group.name}/members.json", params: { limit: 3, offset: 3, asc: true }
 
       expect(response.status).to eq(200)
 
@@ -505,7 +505,7 @@ describe GroupsController do
 
       expect(members.map { |m| m['username'] }).to eq(usernames[3..5])
 
-      get "/groups/#{group.name}/members.json", params: { order: 'added_at', desc: true }
+      get "/groups/#{group.name}/members.json", params: { order: 'added_at' }
       members = response.parsed_body["members"]
 
       expect(members.last['added_at']).to eq(first_user.created_at.as_json)
@@ -851,7 +851,7 @@ describe GroupsController do
 
     it "should allow members to be sorted by" do
       get "/groups/#{group.name}/members.json", params: {
-        order: 'last_seen_at', desc: true
+        order: 'last_seen_at'
       }
 
       expect(response.status).to eq(200)
@@ -860,7 +860,7 @@ describe GroupsController do
 
       expect(members.map { |m| m["id"] }).to eq([user1.id, user2.id, user3.id])
 
-      get "/groups/#{group.name}/members.json", params: { order: 'last_seen_at' }
+      get "/groups/#{group.name}/members.json", params: { order: 'last_seen_at', asc: true }
 
       expect(response.status).to eq(200)
 
@@ -869,7 +869,7 @@ describe GroupsController do
       expect(members.map { |m| m["id"] }).to eq([user2.id, user1.id, user3.id])
 
       get "/groups/#{group.name}/members.json", params: {
-        order: 'last_posted_at', desc: true
+        order: 'last_posted_at'
       }
 
       expect(response.status).to eq(200)

test/javascripts/helpers/create-pretender.js

@@ -620,7 +620,7 @@ export function applyDefaultHandlers(pretender) {
       });
     }
 
-    const ascending = request.queryParams.ascending;
+    const asc = request.queryParams.asc;
     const order = request.queryParams.order;
 
     if (order) {
@@ -629,7 +629,7 @@ export function applyDefaultHandlers(pretender) {
       });
     }
 
-    if (ascending) {
+    if (asc) {
       store = store.reverse();
     }