UX: Allow user_count on groups page to be sortable.

app/assets/javascripts/discourse/components/directory-toggle.js.es6

@@ -1,16 +1,22 @@
 import { iconHTML } from 'discourse-common/lib/icon-library';
 import { bufferedRender } from 'discourse-common/lib/buffered-render';
+import computed from 'ember-addons/ember-computed-decorators';
 
 export default Ember.Component.extend(bufferedRender({
   tagName: 'th',
   classNames: ['sortable'],
   attributeBindings: ['title'],
   rerenderTriggers: ['order', 'asc'],
+  labelKey: null,
+
+  @computed("field", "labelKey")
+  title(field, labelKey) {
+    if (!labelKey) {
+      labelKey = `directory.${this.get('field')}`;
+    }
 
-  title: function() {
-    const labelKey = 'directory.' + this.get('field');
     return I18n.t(labelKey + '_long', { defaultValue: I18n.t(labelKey) });
-  }.property('field'),
+  },
 
   buildBuffer(buffer) {
     const icon = this.get('icon');
@@ -19,7 +25,7 @@ export default Ember.Component.extend(bufferedRender({
     }
 
     const field = this.get('field');
-    buffer.push(I18n.t('directory.' + field));
+    buffer.push(I18n.t(this.get('labelKey') || `directory.${field}`));
 
     if (field === this.get('order')) {
       buffer.push(iconHTML(this.get('asc') ? 'chevron-up' : 'chevron-down'));

app/assets/javascripts/discourse/controllers/groups.js.es6

@@ -2,15 +2,18 @@ import { observes } from 'ember-addons/ember-computed-decorators';
 
 export default Ember.Controller.extend({
   application: Ember.inject.controller(),
+  queryParams: ["order", "asc"],
+  order: null,
+  asc: null,
 
-  @observes("groups.canLoadMore")
+  @observes("model.canLoadMore")
   _showFooter() {
-    this.set("application.showFooter", !this.get("groups.canLoadMore"));
+    this.set("application.showFooter", !this.get("model.canLoadMore"));
   },
 
   actions: {
     loadMore() {
-      this.get('groups').loadMore();
+      this.get('model').loadMore();
     }
   }
 });

app/assets/javascripts/discourse/routes/groups.js.es6

@@ -1,4 +1,11 @@
 export default Discourse.Route.extend({
+  queryParams: {
+    order: { refreshModel: true, replace: true },
+    asc: { refreshModel: true, replace: true },
+  },
+
+  refreshQueryWithoutTransition: true,
+
   titleToken() {
     return I18n.t('groups.index.title');
   },
@@ -8,6 +15,6 @@ export default Discourse.Route.extend({
   },
 
   setupController(controller, model) {
-    controller.set('groups', model);
+    controller.set('model', model);
   }
 });

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

@@ -1,17 +1,18 @@
 {{#d-section pageClass="groups"}}
   <h1>{{i18n "groups.index.title"}}</h1>
-  {{#if groups}}
+
+  {{#if model}}
     {{#load-more selector=".groups-table .groups-table-row" action="loadMore"}}
       <div class='container'>
         <table class="groups-table">
           <thead>
             <th></th>
-            <th>{{i18n "groups.user_count"}}</th>
+            {{directory-toggle field="user_count" labelKey="groups.user_count" order=order asc=asc}}
             <th>{{i18n "groups.membership"}}</th>
           </thead>
 
           <tbody>
-            {{#each groups as |group|}}
+            {{#each model as |group|}}
               <tr class="groups-table-row">
                 <td class="groups-info">
                   {{#link-to "group.members" group.name}}
@@ -61,7 +62,7 @@
       </div>
     {{/load-more}}
 
-    {{conditional-loading-spinner condition=groups.loadingMore}}
+    {{conditional-loading-spinner condition=model.loadingMore}}
   {{else}}
     <p>{{i18n "groups.index.empty"}}</p>
   {{/if}}

app/assets/stylesheets/common/base/groups.scss

@@ -9,16 +9,23 @@
 
   th {
     border-bottom: 1px solid $primary-low;
-    padding: 5px 0;
+    padding: 0.5em;
     text-align: left;
   }
 
+  .sortable {
+    &:hover {
+      background-color: $primary-low;
+      cursor: pointer;
+    }
+  }
+
   tr {
-    border-bottom: 1px solid $primary-low; 
+    border-bottom: 1px solid $primary-low;
 
     td {
       color: blend-primary-secondary(50%);
-      padding: 0.8em 0;
+      padding: 0.8em;
     }
 
     td.groups-user-count {

app/controllers/groups_controller.rb

@@ -20,8 +20,9 @@ class GroupsController < ApplicationController
 
     page_size = 30
     page = params[:page]&.to_i || 0
-
-    groups = Group.visible_groups(current_user)
+    order = %w{name user_count}.delete(params[:order])
+    dir = params[:asc] ? 'ASC' : 'DESC'
+    groups = Group.visible_groups(current_user, order ? "#{order} #{dir}" : nil)
 
     unless guardian.is_staff?
       # hide automatic groups from all non stuff to de-clutter page

app/models/group.rb

@@ -81,8 +81,8 @@ class Group < ActiveRecord::Base
   validates :mentionable_level, inclusion: { in: ALIAS_LEVELS.values }
   validates :messageable_level, inclusion: { in: ALIAS_LEVELS.values }
 
-  scope :visible_groups, ->(user) {
-    groups = Group.order(name: :asc).where("groups.id > 0")
+  scope :visible_groups, Proc.new { |user, order|
+    groups = Group.order(order || "name ASC").where("groups.id > 0")
 
     unless user&.admin
       sql = <<~SQL

spec/requests/groups_controller_spec.rb

@@ -18,6 +18,45 @@ describe GroupsController do
       end
     end
 
+    context 'sortable' do
+      let!(:other_group) { Fabricate(:group, name: "zzzzzz", users: [user]) }
+
+      %w{
+        desc
+        asc
+      }.each do |order|
+        context "#{order} order" do
+          it 'should return the right response' do
+            is_asc = order == 'asc'
+            params = { order: 'name' }
+            params.merge!(asc: true) if is_asc
+            group
+            get "/groups.json", params: params
+
+            expect(response.status).to eq(200)
+
+            group_ids = [group.id, other_group.id]
+            group_ids.reverse! if !is_asc
+
+            expect(JSON.parse(response.body)["groups"].map { |group| group["id"] })
+              .to eq(group_ids)
+          end
+        end
+      end
+
+      context 'ascending order' do
+        it 'should return the right response' do
+          group
+          get "/groups.json", params: { order: 'name' }
+
+          expect(response.status).to eq(200)
+
+          expect(JSON.parse(response.body)["groups"].map { |group| group["id"] })
+            .to eq([other_group.id, group.id])
+        end
+      end
+    end
+
     it 'should return the right response' do
       group
       get "/groups.json"