From 042ab0ad9d039b980547b13f5373771c86c016a8 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 10 Mar 2014 16:21:28 -0700 Subject: [PATCH] Fix three minor edge case behaviors in Conpherence Summary: Couple of tweaks: - If a conpherence has no participants, we fail to `attachParticipants()`. This can happen if you leave a Conpherence as the last participant, then visit the URI again explicitly. - If you can't load any transactions (usually, because you don't have permission to view a thread's transactions), we try to attach `null` instead of `array()`. This can happen if you attempt to view a thread you don't have permission to see. A more general fix would be to tweak the load/filtering order, but I'm leaving that for another time since it's more involved and only gives us a small performance gain in unusual sitautions. - `initializeNewThread()` should be declared `static`. Test Plan: - Viewed a thread with no participants, got proper policy error. - Viewed a thread I couldn't see, got proper policy error. - Grepped for `initializeNewThread()`. Reviewers: btrahan Reviewed By: btrahan CC: aran Differential Revision: https://secure.phabricator.com/D8467 --- .../conpherence/query/ConpherenceThreadQuery.php | 16 ++++++++++++---- .../conpherence/storage/ConpherenceThread.php | 2 +- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/applications/conpherence/query/ConpherenceThreadQuery.php b/src/applications/conpherence/query/ConpherenceThreadQuery.php index de1e8ed7b9..984efa5ba3 100644 --- a/src/applications/conpherence/query/ConpherenceThreadQuery.php +++ b/src/applications/conpherence/query/ConpherenceThreadQuery.php @@ -129,12 +129,20 @@ final class ConpherenceThreadQuery $participants = id(new ConpherenceParticipant()) ->loadAllWhere('conpherencePHID IN (%Ls)', array_keys($conpherences)); $map = mgroup($participants, 'getConpherencePHID'); - foreach ($map as $conpherence_phid => $conpherence_participants) { - $current_conpherence = $conpherences[$conpherence_phid]; + + foreach ($conpherences as $current_conpherence) { + $conpherence_phid = $current_conpherence->getPHID(); + + $conpherence_participants = idx( + $map, + $conpherence_phid, + array()); + $conpherence_participants = mpull( $conpherence_participants, null, 'getParticipantPHID'); + $current_conpherence->attachParticipants($conpherence_participants); $current_conpherence->attachHandles(array()); } @@ -183,13 +191,13 @@ final class ConpherenceThreadQuery $transactions = $query->execute(); $transactions = mgroup($transactions, 'getObjectPHID'); foreach ($conpherences as $phid => $conpherence) { - $current_transactions = $transactions[$phid]; + $current_transactions = idx($transactions, $phid, array()); $handles = array(); foreach ($current_transactions as $transaction) { $handles += $transaction->getHandles(); } $conpherence->attachHandles($conpherence->getHandles() + $handles); - $conpherence->attachTransactions($transactions[$phid]); + $conpherence->attachTransactions($current_transactions); } return $this; } diff --git a/src/applications/conpherence/storage/ConpherenceThread.php b/src/applications/conpherence/storage/ConpherenceThread.php index 0622f9fa9f..3dde719f3b 100644 --- a/src/applications/conpherence/storage/ConpherenceThread.php +++ b/src/applications/conpherence/storage/ConpherenceThread.php @@ -18,7 +18,7 @@ final class ConpherenceThread extends ConpherenceDAO private $widgetData = self::ATTACHABLE; private $images = array(); - public function initializeNewThread(PhabricatorUser $sender) { + public static function initializeNewThread(PhabricatorUser $sender) { return id(new ConpherenceThread()) ->setMessageCount(0) ->setTitle('');