Fix sorting algorithm for group-by-project

Summary:
When viewing Maniphest tasks grouped by project, there's this
weird algorithm that involves generating strings to use as sort keys.
It's pretty clearly wrong in several cases; this aims to fix it.

Test Plan:
Open Maniphest and try to sort by things. Unfortunately, I
don't have access to a decent Maniphest database, so I'm not certain it
works as it should.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, mikaaay

Maniphest Tasks: T1595

Differential Revision: https://secure.phabricator.com/D3142

src/applications/maniphest/ManiphestTaskQuery.php

@@ -72,6 +72,8 @@ final class ManiphestTaskQuery {
 
   private $rowCount         = null;
 
+  private $groupByProjectResults = null; // See comment at bottom for details
+
 
   public function withAuthors(array $authors) {
     $this->authorPHIDs = $authors;
@@ -173,6 +175,10 @@ final class ManiphestTaskQuery {
     return $this->rowCount;
   }
 
+  public function getGroupByProjectResults() {
+    return $this->groupByProjectResults;
+  }
+
   public function withAnyProject($any_project) {
     $this->anyProject = $any_project;
     return $this;
@@ -556,6 +562,12 @@ final class ManiphestTaskQuery {
    * server-side magic since there's currently no way to sort by project name on
    * the database.
    *
+   * As a consequence of this, moreover, because the list we return from here
+   * may include a single task multiple times (once for each project it's in),
+   * sorting gets screwed up in the controller unless we tell it which project
+   * to put the task in each time it appears. Hence the magic field
+   * groupByProjectResults.
+   *
    * TODO: Move this all to the database.
    */
   private function applyGroupByProject(array $tasks) {
@@ -587,8 +599,9 @@ final class ManiphestTaskQuery {
         foreach ($phids as $phid) {
           $items[] = array(
             'key'  => $key,
+            'proj' => $phid,
             'seq'  => sprintf(
-              '%'.$max.'s%d',
+              '%'.$max.'s%09d',
               $handles[$phid]->getName(),
               $ii),
           );
@@ -597,7 +610,11 @@ final class ManiphestTaskQuery {
         // Sort "no project" tasks first.
         $items[] = array(
           'key'  => $key,
-          'seq' => '',
+          'proj' => null,
+          'seq'  => sprintf(
+            '%'.$max.'s%09d',
+            '',
+            $ii),
         );
       }
       ++$ii;
@@ -610,9 +627,11 @@ final class ManiphestTaskQuery {
       nonempty($this->limit, self::DEFAULT_PAGE_SIZE));
 
     $result = array();
+    $projects = array();
     foreach ($items as $item) {
-      $result[] = $tasks[$item['key']];
+      $result[] = $projects[$item['proj']][] = $tasks[$item['key']];
     }
+    $this->groupByProjectResults = $projects;
 
     return $result;
   }

src/applications/maniphest/controller/ManiphestTaskListController.php

@@ -582,20 +582,10 @@ final class ManiphestTaskListController extends ManiphestController {
         break;
       case 'project':
         $grouped = array();
-        foreach ($data as $task) {
+        foreach ($query->getGroupByProjectResults() as $project => $tasks) {
-          $phids = $task->getProjectPHIDs();
+          foreach ($tasks as $task) {
-          if ($project_phids && $any_project !== true) {
+            $group = $project ? $handles[$project]->getName() : 'No Project';
-            // If the user is filtering on "Bugs", don't show a "Bugs" group
+            $grouped[$group][$task->getID()] = $task;
-            // with every result since that's silly (the query also does this
-            // on the backend).
-            $phids = array_diff($phids, $project_phids);
-          }
-          if ($phids) {
-            foreach ($phids as $phid) {
-              $grouped[$handles[$phid]->getName()][$task->getID()] = $task;
-            }
-          } else {
-            $grouped['No Project'][$task->getID()] = $task;
           }
         }
         $data = $grouped;