From 5b8028e254bcfcaf5bdcd71b88afa1f1dcf032ff Mon Sep 17 00:00:00 2001
From: Alan Huang <alanh@fb.com>
Date: Tue, 7 Aug 2012 19:40:38 -0700
Subject: [PATCH] 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
---
 .../maniphest/ManiphestTaskQuery.php          | 31 +++++++++++++++----
 .../ManiphestTaskListController.php           | 18 +++--------
 2 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/src/applications/maniphest/ManiphestTaskQuery.php b/src/applications/maniphest/ManiphestTaskQuery.php
index c67d89ea7d..6ceaa8a5f6 100644
--- a/src/applications/maniphest/ManiphestTaskQuery.php
+++ b/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) {
@@ -586,9 +598,10 @@ final class ManiphestTaskQuery {
       if ($phids) {
         foreach ($phids as $phid) {
           $items[] = array(
-            'key' => $key,
-            'seq' => sprintf(
-              '%'.$max.'s%d',
+            'key'  => $key,
+            'proj' => $phid,
+            'seq'  => sprintf(
+              '%'.$max.'s%09d',
               $handles[$phid]->getName(),
               $ii),
           );
@@ -596,8 +609,12 @@ final class ManiphestTaskQuery {
       } else {
         // Sort "no project" tasks first.
         $items[] = array(
-          'key' => $key,
-          'seq' => '',
+          'key'  => $key,
+          '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;
   }
diff --git a/src/applications/maniphest/controller/ManiphestTaskListController.php b/src/applications/maniphest/controller/ManiphestTaskListController.php
index 7a0d88b74b..3568c7b339 100644
--- a/src/applications/maniphest/controller/ManiphestTaskListController.php
+++ b/src/applications/maniphest/controller/ManiphestTaskListController.php
@@ -582,20 +582,10 @@ final class ManiphestTaskListController extends ManiphestController {
         break;
       case 'project':
         $grouped = array();
-        foreach ($data as $task) {
-          $phids = $task->getProjectPHIDs();
-          if ($project_phids && $any_project !== true) {
-            // If the user is filtering on "Bugs", don't show a "Bugs" group
-            // 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;
+        foreach ($query->getGroupByProjectResults() as $project => $tasks) {
+          foreach ($tasks as $task) {
+            $group = $project ? $handles[$project]->getName() : 'No Project';
+            $grouped[$group][$task->getID()] = $task;
           }
         }
         $data = $grouped;