From 26f7b69ab2f48b10ae110162575ce66e72560ad4 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Fri, 10 Apr 2015 09:08:38 -0700 Subject: [PATCH] Conpherence - fix a fatal Summary: Ref T7795. I can't get this to reproduce and its confusing to me how its possible. The trace in T7795 uses the "LOAD" pathway on the update controller. Under the hood, this issues a ThreadQuery with needTransactions to true. With needTransactions to true, the transactions and pertinent handles are all loaded nicely. So... best guess is there has been some LIMIT of transactions since the offending person participated...? Alternative fix which would probably work is to specify needParticipantCache to true. More on T7795 - the user report found the "a, b, c..." subtitle thing in the messages dropdown confusing. Yet another fix here would be to change that to be something like "a: snippet of what a said...". I'll discuss that on the task. Test Plan: iiam Reviewers: epriestley Reviewed By: epriestley Subscribers: nevogd, Korvin, epriestley Maniphest Tasks: T7795 Differential Revision: https://secure.phabricator.com/D12336 --- .../controller/ConpherenceUpdateController.php | 11 ++++++++--- .../conpherence/query/ConpherenceThreadQuery.php | 6 ++++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/applications/conpherence/controller/ConpherenceUpdateController.php b/src/applications/conpherence/controller/ConpherenceUpdateController.php index 3808b75ee8..5f90df92ba 100644 --- a/src/applications/conpherence/controller/ConpherenceUpdateController.php +++ b/src/applications/conpherence/controller/ConpherenceUpdateController.php @@ -10,6 +10,7 @@ final class ConpherenceUpdateController return new Aphront404Response(); } + $need_participants = false; $needed_capabilities = array(PhabricatorPolicyCapability::CAN_VIEW); $action = $request->getStr('action', ConpherenceUpdateActions::METADATA); switch ($action) { @@ -26,12 +27,17 @@ final class ConpherenceUpdateController case ConpherenceUpdateActions::JOIN_ROOM: $needed_capabilities[] = PhabricatorPolicyCapability::CAN_JOIN; break; + case ConpherenceUpdateActions::NOTIFICATIONS: + $need_participants = true; + break; + case ConpherenceUpdateActions::LOAD: + break; } $conpherence = id(new ConpherenceThreadQuery()) ->setViewer($user) ->withIDs(array($conpherence_id)) ->needFilePHIDs(true) - ->needParticipantCache(true) + ->needParticipants($need_participants) ->requireCapabilities($needed_capabilities) ->executeOne(); @@ -373,10 +379,9 @@ final class ConpherenceUpdateController $need_widget_data = false; $need_transactions = false; - $need_participant_cache = false; + $need_participant_cache = true; switch ($action) { case ConpherenceUpdateActions::METADATA: - $need_participant_cache = true; $need_transactions = true; break; case ConpherenceUpdateActions::LOAD: diff --git a/src/applications/conpherence/query/ConpherenceThreadQuery.php b/src/applications/conpherence/query/ConpherenceThreadQuery.php index 9418f97052..00c2a21460 100644 --- a/src/applications/conpherence/query/ConpherenceThreadQuery.php +++ b/src/applications/conpherence/query/ConpherenceThreadQuery.php @@ -97,7 +97,8 @@ final class ConpherenceThreadQuery $this->loadParticipantsAndInitHandles($conpherences); if ($this->needParticipantCache) { $this->loadCoreHandles($conpherences, 'getRecentParticipantPHIDs'); - } else if ($this->needWidgetData) { + } + if ($this->needWidgetData) { $this->loadCoreHandles($conpherences, 'getParticipantPHIDs'); } if ($this->needTransactions) { @@ -244,7 +245,8 @@ final class ConpherenceThreadQuery ->execute(); foreach ($handle_phids as $conpherence_phid => $phids) { $conpherence = $conpherences[$conpherence_phid]; - $conpherence->attachHandles(array_select_keys($handles, $phids)); + $conpherence->attachHandles( + $conpherence->getHandles() + array_select_keys($handles, $phids)); } return $this; }