From 0292793d4d0524777dc42848b172a555215a3a05 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 11 Aug 2014 12:30:48 -0700 Subject: [PATCH] Account for preempting events on the Phrequent list view Summary: Fixes T5850. Also fixes some logic where the wrong preempting events could be attached during a bulk query. Test Plan: Phrequent list now shows preemption-aware times. Reviewers: hach-que, btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T5850 Differential Revision: https://secure.phabricator.com/D10223 --- .../phrequent/query/PhrequentSearchEngine.php | 16 +++++------ .../query/PhrequentUserTimeQuery.php | 27 ++++++++++++++++--- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/applications/phrequent/query/PhrequentSearchEngine.php b/src/applications/phrequent/query/PhrequentSearchEngine.php index c45fd4a290..0569c93822 100644 --- a/src/applications/phrequent/query/PhrequentSearchEngine.php +++ b/src/applications/phrequent/query/PhrequentSearchEngine.php @@ -29,7 +29,8 @@ final class PhrequentSearchEngine extends PhabricatorApplicationSearchEngine { } public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) { - $query = id(new PhrequentUserTimeQuery()); + $query = id(new PhrequentUserTimeQuery()) + ->needPreemptingEvents(true); $user_phids = $saved->getParameter('userPHIDs'); if ($user_phids) { @@ -136,7 +137,6 @@ final class PhrequentSearchEngine extends PhabricatorApplicationSearchEngine { foreach ($usertimes as $usertime) { $item = new PHUIObjectItemView(); - if ($usertime->getObjectPHID() === null) { $item->setHeader($usertime->getNote()); } else { @@ -154,12 +154,10 @@ final class PhrequentSearchEngine extends PhabricatorApplicationSearchEngine { $started_date = phabricator_date($usertime->getDateStarted(), $viewer); $item->addIcon('none', $started_date); - if ($usertime->getDateEnded() !== null) { - $time_spent = $usertime->getDateEnded() - $usertime->getDateStarted(); - $time_ended = phabricator_datetime($usertime->getDateEnded(), $viewer); - } else { - $time_spent = time() - $usertime->getDateStarted(); - } + $block = new PhrequentTimeBlock(array($usertime)); + $time_spent = $block->getTimeSpentOnObject( + $usertime->getObjectPHID(), + PhabricatorTime::getNow()); $time_spent = $time_spent == 0 ? 'none' : phutil_format_relative_time_detailed($time_spent); @@ -172,7 +170,7 @@ final class PhrequentSearchEngine extends PhabricatorApplicationSearchEngine { $item->addAttribute( pht( 'Ended on %s', - $time_ended)); + phabricator_datetime($usertime->getDateEnded(), $viewer))); } else { $item->addAttribute( pht( diff --git a/src/applications/phrequent/query/PhrequentUserTimeQuery.php b/src/applications/phrequent/query/PhrequentUserTimeQuery.php index c173741c7a..2bb4674ddd 100644 --- a/src/applications/phrequent/query/PhrequentUserTimeQuery.php +++ b/src/applications/phrequent/query/PhrequentUserTimeQuery.php @@ -199,10 +199,31 @@ final class PhrequentUserTimeQuery $u_start = $u_event->getDateStarted(); $u_end = $u_event->getDateEnded(); - if (($u_start >= $e_start) && - ($u_end === null || $u_end > $e_start)) { - $select[] = $u_event; + if ($u_start < $e_start) { + // This event started before our event started, so it's not + // preempting us. + continue; } + + if ($u_start == $e_start) { + if ($u_event->getID() < $event->getID()) { + // This event started at the same time as our event started, + // but has a lower ID, so it's not preempting us. + continue; + } + } + + if (($e_end !== null) && ($u_start > $e_end)) { + // Our event has ended, and this event started after it ended. + continue; + } + + if (($u_end !== null) && ($u_end < $e_start)) { + // This event ended before our event began. + continue; + } + + $select[] = $u_event; } $event->attachPreemptingEvents($select);