Fix a handle-related fatal in Conpherence

I'll file an issue for this, but it locked up my UI since I've got a mention in a recent thread.

Auditors: btrahan

src/applications/conpherence/phid/PhabricatorConpherenceThreadPHIDType.php

@@ -32,10 +32,13 @@ final class PhabricatorConpherenceThreadPHIDType extends PhabricatorPHIDType {
 
     foreach ($handles as $phid => $handle) {
       $thread = $objects[$phid];
-      $data = $thread->getDisplayData($query->getViewer());
-      $handle->setName($data['title']);
-      $handle->setFullName($data['title']);
-      $handle->setURI('/'.$thread->getMonogram());
+
+      $title = $thread->getDisplayTitle($query->getViewer());
+      $monogram = $thread->getMonogram();
+
+      $handle->setName($title);
+      $handle->setFullName(pht('%s: %s', $monogram, $title));
+      $handle->setURI('/'.$monogram);
     }
   }
 

src/applications/conpherence/storage/ConpherenceThread.php

@@ -199,19 +199,76 @@ final class ConpherenceThread extends ConpherenceDAO
     return PhabricatorUser::getDefaultProfileImageURI();
   }
 
-  public function getDisplayData(PhabricatorUser $user) {
+
+  /**
+   * Get the thread's display title for a user.
+   *
+   * If a thread doesn't have a title set, this will return a string describing
+   * recent participants.
+   *
+   * @param PhabricatorUser Viewer.
+   * @return string Thread title.
+   */
+  public function getDisplayTitle(PhabricatorUser $viewer) {
+    $title = $this->getTitle();
+    if (strlen($title)) {
+      return $title;
+    }
+
+    return $this->getRecentParticipantsString($viewer);
+  }
+
+
+  /**
+   * Get recent participants (other than the viewer) as a string.
+   *
+   * For example, this method might return "alincoln, htaft, gwashington...".
+   *
+   * @param PhabricatorUser Viewer.
+   * @return string Description of other participants.
+   */
+  private function getRecentParticipantsString(PhabricatorUser $viewer) {
+    $handles = $this->getHandles();
+    $phids = $this->getOtherRecentParticipantPHIDs($viewer);
+
+    $limit = 3;
+    $more = (count($phids) > $limit);
+    $phids = array_slice($phids, 0, $limit);
+
+    $names = array_select_keys($handles, $phids);
+    $names = mpull($names, 'getName');
+    $names = implode(', ', $names);
+
+    if ($more) {
+      $names = $names.'...';
+    }
+
+    return $names;
+  }
+
+
+  /**
+   * Get PHIDs for recent participants who are not the viewer.
+   *
+   * @param PhabricatorUser Viewer.
+   * @return list<phid> Participants who are not the viewer.
+   */
+  private function getOtherRecentParticipantPHIDs(PhabricatorUser $viewer) {
+    $phids = $this->getRecentParticipantPHIDs();
+    $phids = array_fuse($phids);
+    unset($phids[$viewer->getPHID()]);
+    return array_values($phids);
+  }
+
+
+  public function getDisplayData(PhabricatorUser $viewer) {
+    $handles = $this->getHandles();
+
     if ($this->hasAttachedTransactions()) {
       $transactions = $this->getTransactions();
     } else {
       $transactions = array();
     }
-    $set_title = $this->getTitle();
-
-    if ($set_title) {
-      $title_mode = 'title';
-    } else {
-      $title_mode = 'recent';
-    }
 
     if ($transactions) {
       $subtitle_mode = 'message';
@@ -219,22 +276,7 @@ final class ConpherenceThread extends ConpherenceDAO
       $subtitle_mode = 'recent';
     }
 
-    $recent_phids = $this->getRecentParticipantPHIDs();
-    $handles = $this->getHandles();
-    // Luck has little to do with it really; most recent participant who
-    // isn't the user....
-    $lucky_phid = null;
-    $lucky_index = null;
-    $recent_title = null;
-    foreach ($recent_phids as $index => $phid) {
-      if ($phid == $user->getPHID()) {
-        continue;
-      }
-      $lucky_phid = $phid;
-      break;
-    }
-    reset($recent_phids);
-
+    $lucky_phid = head($this->getOtherRecentParticipantPHIDs($viewer));
     if ($lucky_phid) {
       $lucky_handle = $handles[$lucky_phid];
     } else {
@@ -250,38 +292,6 @@ final class ConpherenceThread extends ConpherenceDAO
       $img_src = $lucky_handle->getImageURI();
     }
 
-    if ($title_mode == 'recent' || $subtitle_mode == 'recent') {
-      $count = 0;
-      $final = false;
-      foreach ($recent_phids as $phid) {
-        if ($phid == $user->getPHID()) {
-          continue;
-        }
-        $handle = $handles[$phid];
-        if ($recent_title) {
-          if ($final) {
-            $recent_title .= '...';
-            break;
-          } else {
-            $recent_title .= ', ';
-          }
-        }
-        $recent_title .= $handle->getName();
-        $count++;
-        $final = $count == 3;
-      }
-    }
-
-    switch ($title_mode) {
-      case 'recent':
-        $title = $recent_title;
-        $js_title = $recent_title;
-        break;
-      case 'title':
-        $title = $js_title = $this->getTitle();
-        break;
-    }
-
     $message_title = null;
     if ($subtitle_mode == 'message') {
       $message_transaction = null;
@@ -307,18 +317,18 @@ final class ConpherenceThread extends ConpherenceDAO
     }
     switch ($subtitle_mode) {
       case 'recent':
-        $subtitle = $recent_title;
+        $subtitle = $this->getRecentParticipantsString($viewer);
         break;
       case 'message':
         if ($message_title) {
           $subtitle = $message_title;
         } else {
-          $subtitle = $recent_title;
+          $subtitle = $this->getRecentParticipantsString($viewer);
         }
         break;
     }
 
-    $user_participation = $this->getParticipantIfExists($user->getPHID());
+    $user_participation = $this->getParticipantIfExists($viewer->getPHID());
     if ($user_participation) {
       $user_seen_count = $user_participation->getSeenMessageCount();
     } else {
@@ -326,9 +336,10 @@ final class ConpherenceThread extends ConpherenceDAO
     }
     $unread_count = $this->getMessageCount() - $user_seen_count;
 
+    $title = $this->getDisplayTitle($viewer);
+
     return array(
       'title' => $title,
-      'js_title' => $js_title,
       'subtitle' => $subtitle,
       'unread_count' => $unread_count,
       'epoch' => $this->getDateModified(),

src/applications/conpherence/view/ConpherenceDurableColumnView.php

@@ -251,7 +251,7 @@ final class ConpherenceDurableColumnView extends AphrontTagView {
         array(),
         array(
           $icon,
-          $data['js_title'],
+          $data['title'],
         ));
       $image = $data['image'];
       Javelin::initBehavior('phabricator-tooltips');
@@ -265,7 +265,7 @@ final class ConpherenceDurableColumnView extends AphrontTagView {
             'meta' => array(
               'threadID' => $conpherence->getID(),
               'threadTitle' => hsprintf('%s', $thread_title),
-              'tip' => $data['js_title'],
+              'tip' => $data['title'],
               'align' => 'S',
             ),
           ),

src/applications/conpherence/view/ConpherenceThreadListView.php

@@ -111,7 +111,7 @@ final class ConpherenceThreadListView extends AphrontView {
       ->addSigil('conpherence-menu-click')
       ->setMetadata(
         array(
-          'title' => $glyph.$data['js_title'],
+          'title' => $glyph.$data['title'],
           'id' => $dom_id,
           'threadID' => $thread->getID(),
           ));