From ab14f49ef87d2b744cbd11cecedad270f4a3f58f Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 15 Nov 2018 05:39:36 -0800
Subject: [PATCH] On the Diffusion cluster status page, improve device sort
 order

Summary:
Ref T13216. See PHI943. When you have a large number of cluster bindings for a repository, the UI sorting can be a bit hard to manage.

One install that regularly cycles repository cluster devices had a couple dozen older disabled bindings, with the enabled bindings intermingled.

Sort the UI:

  - enabled devices come first;
  - in each group, sort by name.

Test Plan: Mixed disabled/enabled bindings, loaded {nav Diffusion > Repository > Storage} page with clustering configured. Before: relatively unhelpful sort order. After: more intuitive sort order.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13216

Differential Revision: https://secure.phabricator.com/D19813
---
 ...fusionRepositoryStorageManagementPanel.php | 33 ++++++++++++++-----
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/src/applications/diffusion/management/DiffusionRepositoryStorageManagementPanel.php b/src/applications/diffusion/management/DiffusionRepositoryStorageManagementPanel.php
index a39dfaa632..0a15a62417 100644
--- a/src/applications/diffusion/management/DiffusionRepositoryStorageManagementPanel.php
+++ b/src/applications/diffusion/management/DiffusionRepositoryStorageManagementPanel.php
@@ -99,15 +99,20 @@ final class DiffusionRepositoryStorageManagementPanel
 
       $versions = mpull($versions, null, 'getDevicePHID');
 
-      foreach ($bindings as $binding_group) {
-        $all_disabled = true;
-        foreach ($binding_group as $binding) {
-          if (!$binding->getIsDisabled()) {
-            $all_disabled = false;
-            break;
-          }
-        }
+      // List enabled devices first, then sort devices in each group by name.
+      $sort = array();
+      foreach ($bindings as $key => $binding_group) {
+        $all_disabled = $this->isDisabledGroup($binding_group);
 
+        $sort[$key] = id(new PhutilSortVector())
+          ->addInt($all_disabled ? 1 : 0)
+          ->addString(head($binding_group)->getDevice()->getName());
+      }
+      $sort = msortv($sort, 'getSelf');
+      $bindings = array_select_keys($bindings, array_keys($sort)) + $bindings;
+
+      foreach ($bindings as $binding_group) {
+        $all_disabled = $this->isDisabledGroup($binding_group);
         $any_binding = head($binding_group);
 
         if ($all_disabled) {
@@ -228,4 +233,16 @@ final class DiffusionRepositoryStorageManagementPanel
     return $this->newBox(pht('Cluster Status'), $table);
   }
 
+  private function isDisabledGroup(array $binding_group) {
+    assert_instances_of($binding_group, 'AlmanacBinding');
+
+    foreach ($binding_group as $binding) {
+      if (!$binding->getIsDisabled()) {
+        return false;
+      }
+    }
+
+    return true;
+  }
+
 }