mirror of
https://we.phorge.it/source/phorge.git
synced 2025-02-01 01:18:22 +01:00
Make Phrequent time accounting aware of the stack
Summary: Ref T3569. Fixes T3567. When figuring out how much time has been spent on an object, subtract "preemptive" events which interrupted the object. Also, make the UI look vaguely sane: {F72773} Test Plan: Added a bunch of unit tests, mucked around in the UI. Reviewers: btrahan Reviewed By: btrahan CC: hach-que, skyronic, aran Maniphest Tasks: T3567, T3569 Differential Revision: https://secure.phabricator.com/D7349
This commit is contained in:
epriestley
parent
82e57ab8a7
commit
943080a4de
6 changed files with 433 additions and 88 deletions
|
|
@ -1958,6 +1958,8 @@ phutil_register_library_map(array(
|
|||
'PhrequentDAO' => 'applications/phrequent/storage/PhrequentDAO.php',
|
||||
'PhrequentListController' => 'applications/phrequent/controller/PhrequentListController.php',
|
||||
'PhrequentSearchEngine' => 'applications/phrequent/query/PhrequentSearchEngine.php',
|
||||
'PhrequentTimeBlock' => 'applications/phrequent/storage/PhrequentTimeBlock.php',
|
||||
'PhrequentTimeBlockTestCase' => 'applications/phrequent/storage/__tests__/PhrequentTimeBlockTestCase.php',
|
||||
'PhrequentTrackController' => 'applications/phrequent/controller/PhrequentTrackController.php',
|
||||
'PhrequentTrackableInterface' => 'applications/phrequent/interface/PhrequentTrackableInterface.php',
|
||||
'PhrequentUIEventListener' => 'applications/phrequent/event/PhrequentUIEventListener.php',
|
||||
|
|
@ -4230,6 +4232,8 @@ phutil_register_library_map(array(
|
|||
1 => 'PhabricatorApplicationSearchResultsControllerInterface',
|
||||
),
|
||||
'PhrequentSearchEngine' => 'PhabricatorApplicationSearchEngine',
|
||||
'PhrequentTimeBlock' => 'Phobject',
|
||||
'PhrequentTimeBlockTestCase' => 'PhabricatorTestCase',
|
||||
'PhrequentTrackController' => 'PhrequentController',
|
||||
'PhrequentUIEventListener' => 'PhutilEventListener',
|
||||
'PhrequentUserTime' =>
|
||||
|
|
|
|||
|
|
@ -61,9 +61,9 @@ final class PhrequentUIEventListener
|
|||
$event->setValue('actions', $actions);
|
||||
}
|
||||
|
||||
private function handlePropertyEvent($event) {
|
||||
$user = $event->getUser();
|
||||
$object = $event->getValue('object');
|
||||
private function handlePropertyEvent($ui_event) {
|
||||
$user = $ui_event->getUser();
|
||||
$object = $ui_event->getValue('object');
|
||||
|
||||
if (!$object || !$object->getPHID()) {
|
||||
// No object, or the object has no PHID yet..
|
||||
|
|
@ -75,51 +75,64 @@ final class PhrequentUIEventListener
|
|||
return;
|
||||
}
|
||||
|
||||
$depth = false;
|
||||
$events = id(new PhrequentUserTimeQuery())
|
||||
->setViewer($user)
|
||||
->withObjectPHIDs(array($object->getPHID()))
|
||||
->needPreemptingEvents(true)
|
||||
->execute();
|
||||
$event_groups = mgroup($events, 'getUserPHID');
|
||||
|
||||
$stack = PhrequentUserTimeQuery::loadUserStack($user);
|
||||
if ($stack) {
|
||||
$stack = array_values($stack);
|
||||
for ($ii = 0; $ii < count($stack); $ii++) {
|
||||
if ($stack[$ii]->getObjectPHID() == $object->getPHID()) {
|
||||
$depth = ($ii + 1);
|
||||
if (!$events) {
|
||||
return;
|
||||
}
|
||||
|
||||
$handles = id(new PhabricatorHandleQuery())
|
||||
->setViewer($user)
|
||||
->withPHIDs(array_keys($event_groups))
|
||||
->execute();
|
||||
|
||||
$status_view = new PHUIStatusListView();
|
||||
|
||||
foreach ($event_groups as $user_phid => $event_group) {
|
||||
$item = new PHUIStatusItemView();
|
||||
$item->setTarget($handles[$user_phid]->renderLink());
|
||||
|
||||
$state = 'stopped';
|
||||
foreach ($event_group as $event) {
|
||||
if ($event->getDateEnded() === null) {
|
||||
if ($event->isPreempted()) {
|
||||
$state = 'suspended';
|
||||
} else {
|
||||
$state = 'active';
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
$time_spent = PhrequentUserTimeQuery::getTotalTimeSpentOnObject(
|
||||
$object->getPHID());
|
||||
|
||||
if (!$depth && !$time_spent) {
|
||||
return;
|
||||
switch ($state) {
|
||||
case 'active':
|
||||
$item->setIcon('time-green', pht('Working Now'));
|
||||
break;
|
||||
case 'suspended':
|
||||
$item->setIcon('time-yellow', pht('Interrupted'));
|
||||
break;
|
||||
case 'stopped':
|
||||
$item->setIcon('time-orange', pht('Not Working Now'));
|
||||
break;
|
||||
}
|
||||
|
||||
require_celerity_resource('phrequent-css');
|
||||
$block = new PhrequentTimeBlock($event_group);
|
||||
$item->setNote(
|
||||
phabricator_format_relative_time(
|
||||
$block->getTimeSpentOnObject(
|
||||
$object->getPHID(),
|
||||
time())));
|
||||
|
||||
$property = array();
|
||||
if ($depth == 1) {
|
||||
$property[] = phutil_tag(
|
||||
'div',
|
||||
array(
|
||||
'class' => 'phrequent-tracking-property phrequent-active',
|
||||
),
|
||||
pht('Currently Tracking'));
|
||||
} else if ($depth > 1) {
|
||||
$property[] = phutil_tag(
|
||||
'div',
|
||||
array(
|
||||
'class' => 'phrequent-tracking-property phrequent-on-stack',
|
||||
),
|
||||
pht('On Stack'));
|
||||
$status_view->addItem($item);
|
||||
}
|
||||
|
||||
if ($time_spent) {
|
||||
$property[] = phabricator_format_relative_time_detailed($time_spent);
|
||||
}
|
||||
|
||||
$view = $event->getValue('view');
|
||||
$view->addProperty(pht('Time Spent'), $property);
|
||||
$view = $ui_event->getValue('view');
|
||||
$view->addProperty(pht('Time Spent'), $status_view);
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -21,6 +21,8 @@ final class PhrequentUserTimeQuery
|
|||
private $order = self::ORDER_ID_ASC;
|
||||
private $ended = self::ENDED_ALL;
|
||||
|
||||
private $needPreemptingEvents;
|
||||
|
||||
public function withUserPHIDs($user_phids) {
|
||||
$this->userPHIDs = $user_phids;
|
||||
return $this;
|
||||
|
|
@ -41,6 +43,11 @@ final class PhrequentUserTimeQuery
|
|||
return $this;
|
||||
}
|
||||
|
||||
public function needPreemptingEvents($need_events) {
|
||||
$this->needPreemptingEvents = $need_events;
|
||||
return $this;
|
||||
}
|
||||
|
||||
private function buildWhereClause(AphrontDatabaseConnection $conn) {
|
||||
$where = array();
|
||||
|
||||
|
|
@ -150,6 +157,61 @@ final class PhrequentUserTimeQuery
|
|||
return $usertime->loadAllFromArray($data);
|
||||
}
|
||||
|
||||
protected function didFilterPage(array $page) {
|
||||
if ($this->needPreemptingEvents) {
|
||||
$usertime = new PhrequentUserTime();
|
||||
$conn_r = $usertime->establishConnection('r');
|
||||
|
||||
$preempt = array();
|
||||
foreach ($page as $event) {
|
||||
$preempt[] = qsprintf(
|
||||
$conn_r,
|
||||
'(userPHID = %s AND
|
||||
(dateStarted BETWEEN %d AND %d) AND
|
||||
(dateEnded IS NULL OR dateEnded > %d))',
|
||||
$event->getUserPHID(),
|
||||
$event->getDateStarted(),
|
||||
nonempty($event->getDateEnded(), PhabricatorTime::getNow()),
|
||||
$event->getDateStarted());
|
||||
}
|
||||
|
||||
$preempting_events = queryfx_all(
|
||||
$conn_r,
|
||||
'SELECT * FROM %T WHERE %Q ORDER BY dateStarted ASC, id ASC',
|
||||
$usertime->getTableName(),
|
||||
implode(' OR ', $preempt));
|
||||
$preempting_events = $usertime->loadAllFromArray($preempting_events);
|
||||
|
||||
$preempting_events = mgroup($preempting_events, 'getUserPHID');
|
||||
|
||||
foreach ($page as $event) {
|
||||
$e_start = $event->getDateStarted();
|
||||
$e_end = $event->getDateEnded();
|
||||
|
||||
$select = array();
|
||||
$user_events = idx($preempting_events, $event->getUserPHID(), array());
|
||||
foreach ($user_events as $u_event) {
|
||||
if ($u_event->getID() == $event->getID()) {
|
||||
// Don't allow an event to preempt itself.
|
||||
continue;
|
||||
}
|
||||
|
||||
$u_start = $u_event->getDateStarted();
|
||||
$u_end = $u_event->getDateEnded();
|
||||
|
||||
if (($u_start >= $e_start) && ($u_end <= $e_end) &&
|
||||
($u_end === null || $u_end > $e_start)) {
|
||||
$select[] = $u_event;
|
||||
}
|
||||
}
|
||||
|
||||
$event->attachPreemptingEvents($select);
|
||||
}
|
||||
}
|
||||
|
||||
return $page;
|
||||
}
|
||||
|
||||
/* -( Helper Functions ) --------------------------------------------------- */
|
||||
|
||||
public static function getEndedSearchOptions() {
|
||||
|
|
@ -204,46 +266,6 @@ final class PhrequentUserTimeQuery
|
|||
return $count['N'] > 0;
|
||||
}
|
||||
|
||||
public static function loadUserStack(PhabricatorUser $user) {
|
||||
if (!$user->isLoggedIn()) {
|
||||
return array();
|
||||
}
|
||||
|
||||
return id(new PhrequentUserTime())->loadAllWhere(
|
||||
'userPHID = %s AND dateEnded IS NULL
|
||||
ORDER BY dateStarted DESC, id DESC',
|
||||
$user->getPHID());
|
||||
}
|
||||
|
||||
public static function getTotalTimeSpentOnObject($phid) {
|
||||
$usertime_dao = new PhrequentUserTime();
|
||||
$conn = $usertime_dao->establishConnection('r');
|
||||
|
||||
// First calculate all the time spent where the
|
||||
// usertime blocks have ended.
|
||||
$sum_ended = queryfx_one(
|
||||
$conn,
|
||||
'SELECT SUM(usertime.dateEnded - usertime.dateStarted) N '.
|
||||
'FROM %T usertime '.
|
||||
'WHERE usertime.objectPHID = %s '.
|
||||
'AND usertime.dateEnded IS NOT NULL',
|
||||
$usertime_dao->getTableName(),
|
||||
$phid);
|
||||
|
||||
// Now calculate the time spent where the usertime
|
||||
// blocks have not yet ended.
|
||||
$sum_not_ended = queryfx_one(
|
||||
$conn,
|
||||
'SELECT SUM(UNIX_TIMESTAMP() - usertime.dateStarted) N '.
|
||||
'FROM %T usertime '.
|
||||
'WHERE usertime.objectPHID = %s '.
|
||||
'AND usertime.dateEnded IS NULL',
|
||||
$usertime_dao->getTableName(),
|
||||
$phid);
|
||||
|
||||
return $sum_ended['N'] + $sum_not_ended['N'];
|
||||
}
|
||||
|
||||
public static function getUserTimeSpentOnObject(
|
||||
PhabricatorUser $user,
|
||||
$phid) {
|
||||
|
|
|
|||
155
src/applications/phrequent/storage/PhrequentTimeBlock.php
Normal file
155
src/applications/phrequent/storage/PhrequentTimeBlock.php
Normal file
|
|
@ -0,0 +1,155 @@
|
|||
<?php
|
||||
|
||||
final class PhrequentTimeBlock extends Phobject {
|
||||
|
||||
private $events;
|
||||
|
||||
public function __construct(array $events) {
|
||||
assert_instances_of($events, 'PhrequentUserTime');
|
||||
$this->events = $events;
|
||||
}
|
||||
|
||||
public function getTimeSpentOnObject($phid, $now) {
|
||||
$ranges = idx($this->getObjectTimeRanges($now), $phid, array());
|
||||
|
||||
$sum = 0;
|
||||
foreach ($ranges as $range) {
|
||||
$sum += ($range[1] - $range[0]);
|
||||
}
|
||||
|
||||
return $sum;
|
||||
}
|
||||
|
||||
public function getObjectTimeRanges($now) {
|
||||
$ranges = array();
|
||||
|
||||
$object_ranges = array();
|
||||
foreach ($this->events as $event) {
|
||||
|
||||
// First, convert each event's preempting stack into a linear timeline
|
||||
// of events.
|
||||
|
||||
$timeline = array();
|
||||
$timeline[] = array(
|
||||
'at' => $event->getDateStarted(),
|
||||
'type' => 'start',
|
||||
);
|
||||
$timeline[] = array(
|
||||
'at' => nonempty($event->getDateEnded(), $now),
|
||||
'type' => 'end',
|
||||
);
|
||||
|
||||
$base_phid = $event->getObjectPHID();
|
||||
|
||||
$preempts = $event->getPreemptingEvents();
|
||||
|
||||
foreach ($preempts as $preempt) {
|
||||
$same_object = ($preempt->getObjectPHID() == $base_phid);
|
||||
$timeline[] = array(
|
||||
'at' => $preempt->getDateStarted(),
|
||||
'type' => $same_object ? 'start' : 'push',
|
||||
);
|
||||
$timeline[] = array(
|
||||
'at' => nonempty($preempt->getDateEnded(), $now),
|
||||
'type' => $same_object ? 'end' : 'pop',
|
||||
);
|
||||
}
|
||||
|
||||
// Now, figure out how much time was actually spent working on the
|
||||
// object.
|
||||
|
||||
$timeline = isort($timeline, 'at');
|
||||
|
||||
$stack = array();
|
||||
$depth = null;
|
||||
|
||||
$ranges = array();
|
||||
foreach ($timeline as $timeline_event) {
|
||||
switch ($timeline_event['type']) {
|
||||
case 'start':
|
||||
$stack[] = $depth;
|
||||
$depth = 0;
|
||||
$range_start = $timeline_event['at'];
|
||||
break;
|
||||
case 'end':
|
||||
if ($depth == 0) {
|
||||
$ranges[] = array($range_start, $timeline_event['at']);
|
||||
}
|
||||
$depth = array_pop($stack);
|
||||
break;
|
||||
case 'push':
|
||||
if ($depth == 0) {
|
||||
$ranges[] = array($range_start, $timeline_event['at']);
|
||||
}
|
||||
$depth++;
|
||||
break;
|
||||
case 'pop':
|
||||
$depth--;
|
||||
if ($depth == 0) {
|
||||
$range_start = $timeline_event['at'];
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
$object_ranges[$base_phid][] = $ranges;
|
||||
}
|
||||
|
||||
// Finally, collapse all the ranges so we don't double-count time.
|
||||
|
||||
foreach ($object_ranges as $phid => $ranges) {
|
||||
$object_ranges[$phid] = self::mergeTimeRanges(array_mergev($ranges));
|
||||
}
|
||||
|
||||
return $object_ranges;
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Merge a list of time ranges (pairs of `<start, end>` epochs) so that no
|
||||
* elements overlap. For example, the ranges:
|
||||
*
|
||||
* array(
|
||||
* array(50, 150),
|
||||
* array(100, 175),
|
||||
* );
|
||||
*
|
||||
* ...are merged to:
|
||||
*
|
||||
* array(
|
||||
* array(50, 175),
|
||||
* );
|
||||
*
|
||||
* This is used to avoid double-counting time on objects which had timers
|
||||
* started multiple times.
|
||||
*
|
||||
* @param list<pair<int, int>> List of possibly overlapping time ranges.
|
||||
* @return list<pair<int, int>> Nonoverlapping time ranges.
|
||||
*/
|
||||
public static function mergeTimeRanges(array $ranges) {
|
||||
$ranges = isort($ranges, 0);
|
||||
|
||||
$result = array();
|
||||
|
||||
$current = null;
|
||||
foreach ($ranges as $key => $range) {
|
||||
if ($current === null) {
|
||||
$current = $range;
|
||||
continue;
|
||||
}
|
||||
|
||||
if ($range[0] <= $current[1]) {
|
||||
$current[1] = max($range[1], $current[1]);
|
||||
continue;
|
||||
}
|
||||
|
||||
$result[] = $current;
|
||||
$current = $range;
|
||||
}
|
||||
|
||||
$result[] = $current;
|
||||
|
||||
return $result;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
@ -1,8 +1,5 @@
|
|||
<?php
|
||||
|
||||
/**
|
||||
* @group phrequent
|
||||
*/
|
||||
final class PhrequentUserTime extends PhrequentDAO
|
||||
implements PhabricatorPolicyInterface {
|
||||
|
||||
|
|
@ -12,6 +9,8 @@ final class PhrequentUserTime extends PhrequentDAO
|
|||
protected $dateStarted;
|
||||
protected $dateEnded;
|
||||
|
||||
private $preemptingEvents = self::ATTACHABLE;
|
||||
|
||||
public function getCapabilities() {
|
||||
return array(
|
||||
PhabricatorPolicyCapability::CAN_VIEW,
|
||||
|
|
@ -23,8 +22,11 @@ final class PhrequentUserTime extends PhrequentDAO
|
|||
|
||||
switch ($capability) {
|
||||
case PhabricatorPolicyCapability::CAN_VIEW:
|
||||
$policy = PhabricatorPolicies::POLICY_USER;
|
||||
break;
|
||||
// Since it's impossible to perform any meaningful computations with
|
||||
// time if a user can't view some of it, visibility on tracked time is
|
||||
// unrestricted. If we eventually lock it down, it should be per-user.
|
||||
// (This doesn't mean that users can see tracked objects.)
|
||||
return PhabricatorPolicies::getMostOpenPolicy();
|
||||
}
|
||||
|
||||
return $policy;
|
||||
|
|
@ -36,8 +38,29 @@ final class PhrequentUserTime extends PhrequentDAO
|
|||
|
||||
|
||||
public function describeAutomaticCapability($capability) {
|
||||
return pht(
|
||||
'The user who tracked time can always view it.');
|
||||
return null;
|
||||
}
|
||||
|
||||
public function attachPreemptingEvents(array $events) {
|
||||
$this->preemptingEvents = $events;
|
||||
return $this;
|
||||
}
|
||||
|
||||
public function getPreemptingEvents() {
|
||||
return $this->assertAttached($this->preemptingEvents);
|
||||
}
|
||||
|
||||
public function isPreempted() {
|
||||
if ($this->getDateEnded() !== null) {
|
||||
return false;
|
||||
}
|
||||
foreach ($this->getPreemptingEvents() as $event) {
|
||||
if ($event->getDateEnded() === null &&
|
||||
$event->getObjectPHID() != $this->getObjectPHID()) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,128 @@
|
|||
<?php
|
||||
|
||||
final class PhrequentTimeBlockTestCase extends PhabricatorTestCase {
|
||||
|
||||
public function testMergeTimeRanges() {
|
||||
|
||||
// Overlapping ranges.
|
||||
$input = array(
|
||||
array(50, 150),
|
||||
array(100, 175),
|
||||
);
|
||||
$expect = array(
|
||||
array(50, 175),
|
||||
);
|
||||
|
||||
$this->assertEqual($expect, PhrequentTimeBlock::mergeTimeRanges($input));
|
||||
|
||||
|
||||
// Identical ranges.
|
||||
$input = array(
|
||||
array(1, 1),
|
||||
array(1, 1),
|
||||
array(1, 1),
|
||||
);
|
||||
$expect = array(
|
||||
array(1, 1),
|
||||
);
|
||||
|
||||
$this->assertEqual($expect, PhrequentTimeBlock::mergeTimeRanges($input));
|
||||
|
||||
|
||||
// Range which is a strict subset of another range.
|
||||
$input = array(
|
||||
array(2, 7),
|
||||
array(1, 10),
|
||||
);
|
||||
$expect = array(
|
||||
array(1, 10),
|
||||
);
|
||||
|
||||
$this->assertEqual($expect, PhrequentTimeBlock::mergeTimeRanges($input));
|
||||
|
||||
|
||||
// These are discontinuous and should not be merged.
|
||||
$input = array(
|
||||
array(5, 6),
|
||||
array(7, 8),
|
||||
);
|
||||
$expect = array(
|
||||
array(5, 6),
|
||||
array(7, 8),
|
||||
);
|
||||
|
||||
$this->assertEqual($expect, PhrequentTimeBlock::mergeTimeRanges($input));
|
||||
|
||||
|
||||
// These overlap only on an edge, but should merge.
|
||||
$input = array(
|
||||
array(5, 6),
|
||||
array(6, 7),
|
||||
);
|
||||
$expect = array(
|
||||
array(5, 7),
|
||||
);
|
||||
|
||||
$this->assertEqual($expect, PhrequentTimeBlock::mergeTimeRanges($input));
|
||||
}
|
||||
|
||||
public function testPreemptingEvents() {
|
||||
|
||||
// Roughly, this is: got into work, started T1, had a meeting from 10-11,
|
||||
// left the office around 2 to meet a client at a coffee shop, worked on
|
||||
// T1 again for about 40 minutes, had the meeting from 3-3:30, finished up
|
||||
// on T1, headed back to the office, got another hour of work in, and
|
||||
// headed home.
|
||||
|
||||
$event = $this->newEvent('T1', 900, 1700);
|
||||
|
||||
$event->attachPreemptingEvents(
|
||||
array(
|
||||
$this->newEvent('meeting', 1000, 1100),
|
||||
$this->newEvent('offsite', 1400, 1600),
|
||||
$this->newEvent('T1', 1420, 1580),
|
||||
$this->newEvent('offsite meeting', 1500, 1550),
|
||||
));
|
||||
|
||||
$block = new PhrequentTimeBlock(array($event));
|
||||
|
||||
$ranges = $block->getObjectTimeRanges(1800);
|
||||
$this->assertEqual(
|
||||
array(
|
||||
'T1' => array(
|
||||
array(900, 1000), // Before morning meeting.
|
||||
array(1100, 1400), // After morning meeting.
|
||||
array(1420, 1500), // Coffee, before client meeting.
|
||||
array(1550, 1580), // Coffee, after client meeting.
|
||||
array(1600, 1700), // After returning from off site.
|
||||
),
|
||||
),
|
||||
$ranges);
|
||||
|
||||
|
||||
$event = $this->newEvent('T2', 100, 300);
|
||||
$event->attachPreemptingEvents(
|
||||
array(
|
||||
$this->newEvent('meeting', 200, null),
|
||||
));
|
||||
|
||||
$block = new PhrequentTimeBlock(array($event));
|
||||
|
||||
$ranges = $block->getObjectTimeRanges(1000);
|
||||
$this->assertEqual(
|
||||
array(
|
||||
'T2' => array(
|
||||
array(100, 200),
|
||||
),
|
||||
),
|
||||
$ranges);
|
||||
}
|
||||
|
||||
private function newEvent($object_phid, $start_time, $end_time) {
|
||||
return id(new PhrequentUserTime())
|
||||
->setObjectPHID($object_phid)
|
||||
->setDateStarted($start_time)
|
||||
->setDateEnded($end_time);
|
||||
}
|
||||
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue