Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15406857
D19122.id45821.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
14 KB
Referenced Files
None
Subscribers
None
D19122.id45821.diff
View Options
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -2928,7 +2928,6 @@
'PhabricatorFactManagementDestroyWorkflow' => 'applications/fact/management/PhabricatorFactManagementDestroyWorkflow.php',
'PhabricatorFactManagementListWorkflow' => 'applications/fact/management/PhabricatorFactManagementListWorkflow.php',
'PhabricatorFactManagementWorkflow' => 'applications/fact/management/PhabricatorFactManagementWorkflow.php',
- 'PhabricatorFactManiphestTaskEngine' => 'applications/fact/engine/PhabricatorFactManiphestTaskEngine.php',
'PhabricatorFactObjectController' => 'applications/fact/controller/PhabricatorFactObjectController.php',
'PhabricatorFactObjectDimension' => 'applications/fact/storage/PhabricatorFactObjectDimension.php',
'PhabricatorFactRaw' => 'applications/fact/storage/PhabricatorFactRaw.php',
@@ -3264,6 +3263,7 @@
'PhabricatorManagementWorkflow' => 'infrastructure/management/PhabricatorManagementWorkflow.php',
'PhabricatorManiphestApplication' => 'applications/maniphest/application/PhabricatorManiphestApplication.php',
'PhabricatorManiphestConfigOptions' => 'applications/maniphest/config/PhabricatorManiphestConfigOptions.php',
+ 'PhabricatorManiphestTaskFactEngine' => 'applications/fact/engine/PhabricatorManiphestTaskFactEngine.php',
'PhabricatorManiphestTaskTestDataGenerator' => 'applications/maniphest/lipsum/PhabricatorManiphestTaskTestDataGenerator.php',
'PhabricatorManualActivitySetupCheck' => 'applications/config/check/PhabricatorManualActivitySetupCheck.php',
'PhabricatorMarkupCache' => 'applications/cache/storage/PhabricatorMarkupCache.php',
@@ -4362,6 +4362,7 @@
'PhabricatorTokensSettingsPanel' => 'applications/settings/panel/PhabricatorTokensSettingsPanel.php',
'PhabricatorTokensToken' => 'applications/tokens/storage/PhabricatorTokensToken.php',
'PhabricatorTransactionChange' => 'applications/transactions/data/PhabricatorTransactionChange.php',
+ 'PhabricatorTransactionFactEngine' => 'applications/fact/engine/PhabricatorTransactionFactEngine.php',
'PhabricatorTransactionRemarkupChange' => 'applications/transactions/data/PhabricatorTransactionRemarkupChange.php',
'PhabricatorTransactions' => 'applications/transactions/constants/PhabricatorTransactions.php',
'PhabricatorTransactionsApplication' => 'applications/transactions/application/PhabricatorTransactionsApplication.php',
@@ -8459,7 +8460,6 @@
'PhabricatorFactManagementDestroyWorkflow' => 'PhabricatorFactManagementWorkflow',
'PhabricatorFactManagementListWorkflow' => 'PhabricatorFactManagementWorkflow',
'PhabricatorFactManagementWorkflow' => 'PhabricatorManagementWorkflow',
- 'PhabricatorFactManiphestTaskEngine' => 'PhabricatorFactEngine',
'PhabricatorFactObjectController' => 'PhabricatorFactController',
'PhabricatorFactObjectDimension' => 'PhabricatorFactDimension',
'PhabricatorFactRaw' => 'PhabricatorFactDAO',
@@ -8833,6 +8833,7 @@
'PhabricatorManagementWorkflow' => 'PhutilArgumentWorkflow',
'PhabricatorManiphestApplication' => 'PhabricatorApplication',
'PhabricatorManiphestConfigOptions' => 'PhabricatorApplicationConfigOptions',
+ 'PhabricatorManiphestTaskFactEngine' => 'PhabricatorTransactionFactEngine',
'PhabricatorManiphestTaskTestDataGenerator' => 'PhabricatorTestDataGenerator',
'PhabricatorManualActivitySetupCheck' => 'PhabricatorSetupCheck',
'PhabricatorMarkupCache' => 'PhabricatorCacheDAO',
@@ -10156,6 +10157,7 @@
'PhabricatorConduitResultInterface',
),
'PhabricatorTransactionChange' => 'Phobject',
+ 'PhabricatorTransactionFactEngine' => 'PhabricatorFactEngine',
'PhabricatorTransactionRemarkupChange' => 'PhabricatorTransactionChange',
'PhabricatorTransactions' => 'Phobject',
'PhabricatorTransactionsApplication' => 'PhabricatorApplication',
diff --git a/src/applications/fact/controller/PhabricatorFactObjectController.php b/src/applications/fact/controller/PhabricatorFactObjectController.php
--- a/src/applications/fact/controller/PhabricatorFactObjectController.php
+++ b/src/applications/fact/controller/PhabricatorFactObjectController.php
@@ -17,6 +17,10 @@
$engines = PhabricatorFactEngine::loadAllEngines();
foreach ($engines as $key => $engine) {
+ $engine = id(clone $engine)
+ ->setViewer($viewer);
+ $engines[$key] = $engine;
+
if (!$engine->supportsDatapointsForObject($object)) {
unset($engines[$key]);
}
@@ -250,6 +254,58 @@
->setTable($table);
$content[] = $box;
+
+ if ($engine instanceof PhabricatorTransactionFactEngine) {
+ $groups = $engine->newTransactionGroupsForObject($object);
+ $groups = array_values($groups);
+
+ $xaction_phids = array();
+ foreach ($groups as $group_key => $xactions) {
+ foreach ($xactions as $xaction) {
+ $xaction_phids[] = $xaction->getAuthorPHID();
+ }
+ }
+ $xaction_handles = $viewer->loadHandles($xaction_phids);
+
+ $rows = array();
+ foreach ($groups as $group_key => $xactions) {
+ foreach ($xactions as $xaction) {
+ $rows[] = array(
+ $group_key,
+ $xaction->getTransactionType(),
+ $xaction_handles[$xaction->getAuthorPHID()]->renderLink(),
+ phabricator_datetime($xaction->getDateCreated(), $viewer),
+ );
+ }
+ }
+
+ $table = id(new AphrontTableView($rows))
+ ->setHeaders(
+ array(
+ pht('Group'),
+ pht('Type'),
+ pht('Author'),
+ pht('Date'),
+ ))
+ ->setColumnClasses(
+ array(
+ null,
+ 'pri',
+ 'wide',
+ 'right',
+ ));
+
+ $header = pht(
+ '%s (Transactions)',
+ get_class($engine));
+
+ $xaction_box = id(new PHUIObjectBoxView())
+ ->setHeaderText($header)
+ ->setTable($table);
+
+ $content[] = $xaction_box;
+ }
+
}
$crumbs = $this->buildApplicationCrumbs()
diff --git a/src/applications/fact/daemon/PhabricatorFactDaemon.php b/src/applications/fact/daemon/PhabricatorFactDaemon.php
--- a/src/applications/fact/daemon/PhabricatorFactDaemon.php
+++ b/src/applications/fact/daemon/PhabricatorFactDaemon.php
@@ -62,6 +62,11 @@
public function setEngines(array $engines) {
assert_instances_of($engines, 'PhabricatorFactEngine');
+ $viewer = PhabricatorUser::getOmnipotentUser();
+ foreach ($engines as $engine) {
+ $engine->setViewer($viewer);
+ }
+
$this->engines = $engines;
return $this;
}
diff --git a/src/applications/fact/engine/PhabricatorFactEngine.php b/src/applications/fact/engine/PhabricatorFactEngine.php
--- a/src/applications/fact/engine/PhabricatorFactEngine.php
+++ b/src/applications/fact/engine/PhabricatorFactEngine.php
@@ -3,6 +3,7 @@
abstract class PhabricatorFactEngine extends Phobject {
private $factMap;
+ private $viewer;
final public static function loadAllEngines() {
return id(new PhutilClassMapQuery())
@@ -35,8 +36,17 @@
return $this->factMap[$key];
}
- final protected function getViewer() {
- return PhabricatorUser::getOmnipotentUser();
+ public function setViewer(PhabricatorUser $viewer) {
+ $this->viewer = $viewer;
+ return $this;
+ }
+
+ public function getViewer() {
+ if (!$this->viewer) {
+ throw new PhutilInvalidStateException('setViewer');
+ }
+
+ return $this->viewer;
}
}
diff --git a/src/applications/fact/engine/PhabricatorFactManiphestTaskEngine.php b/src/applications/fact/engine/PhabricatorManiphestTaskFactEngine.php
rename from src/applications/fact/engine/PhabricatorFactManiphestTaskEngine.php
rename to src/applications/fact/engine/PhabricatorManiphestTaskFactEngine.php
--- a/src/applications/fact/engine/PhabricatorFactManiphestTaskEngine.php
+++ b/src/applications/fact/engine/PhabricatorManiphestTaskFactEngine.php
@@ -1,7 +1,7 @@
<?php
-final class PhabricatorFactManiphestTaskEngine
- extends PhabricatorFactEngine {
+final class PhabricatorManiphestTaskFactEngine
+ extends PhabricatorTransactionFactEngine {
public function newFacts() {
return array(
@@ -73,7 +73,7 @@
id(new PhabricatorPointsFact())
->setKey('tasks.open-points.status.owner'),
id(new PhabricatorPointsFact())
- ->setKey('tasks.open-points.score.project'),
+ ->setKey('tasks.open-points.score.owner'),
id(new PhabricatorPointsFact())
->setKey('tasks.open-points.assign.owner'),
);
@@ -84,16 +84,7 @@
}
public function newDatapointsForObject(PhabricatorLiskDAO $object) {
- $viewer = $this->getViewer();
-
- $xaction_query = PhabricatorApplicationTransactionQuery::newQueryForObject(
- $object);
- $xactions = $xaction_query
- ->setViewer($viewer)
- ->withObjectPHIDs(array($object->getPHID()))
- ->execute();
-
- $xactions = msortv($xactions, 'newChronologicalSortVector');
+ $xaction_groups = $this->newTransactionGroupsForObject($object);
$old_open = false;
$old_points = 0;
@@ -104,7 +95,7 @@
$specs = array();
$datapoints = array();
- foreach ($xactions as $xaction_group) {
+ foreach ($xaction_groups as $xaction_group) {
$add_projects = array();
$rem_projects = array();
@@ -112,8 +103,11 @@
$new_points = $old_points;
$new_owner = $old_owner;
- // TODO: Actually group concurrent transactions.
- $xaction_group = array($xaction_group);
+ if ($is_create) {
+ // Assume tasks start open.
+ // TODO: This might be a questionable assumption?
+ $new_open = true;
+ }
$group_epoch = last($xaction_group)->getDateCreated();
foreach ($xaction_group as $xaction) {
@@ -167,15 +161,17 @@
if ($is_create) {
$action = 'create';
$action_points = $new_points;
+ $include_open = $new_open;
} else {
$action = 'assign';
$action_points = $old_points;
+ $include_open = $old_open;
}
foreach ($project_sets as $project_set) {
$scale = $project_set['scale'];
foreach ($project_set['phids'] as $project_phid) {
- if ($old_open) {
+ if ($include_open) {
$specs[] = array(
"tasks.open-count.{$action}.project",
1 * $scale,
@@ -227,7 +223,9 @@
continue;
}
- if ($old_open) {
+ $scale = $owner_set['scale'];
+
+ if ($old_open != $new_open) {
$specs[] = array(
"tasks.open-count.{$action}.owner",
1 * $scale,
@@ -247,14 +245,14 @@
$owner_phid,
);
- $specs[] = array(
- "tasks.points.{$action}.owner",
- $action_points * $scale,
- $owner_phid,
- );
+ if ($action_points) {
+ $specs[] = array(
+ "tasks.points.{$action}.owner",
+ $action_points * $scale,
+ $owner_phid,
+ );
+ }
}
-
- $old_owner = $new_owner;
}
if ($is_create) {
@@ -262,6 +260,7 @@
'tasks.count.create',
1,
);
+
$specs[] = array(
'tasks.points.create',
$new_points,
@@ -316,11 +315,9 @@
$specs[] = array(
'tasks.open-points.status.project',
$action_points * $scale,
- $new_owner,
+ $project_phid,
);
}
-
- $old_open = $new_open;
}
// The "score" facts only apply to rescoring tasks which already
@@ -371,14 +368,23 @@
$delta,
);
}
-
- $old_points = $new_points;
}
+ $old_points = $new_points;
+ $old_open = $new_open;
+ $old_owner = $new_owner;
+
foreach ($specs as $spec) {
$spec_key = $spec[0];
$spec_value = $spec[1];
+ // Don't write any facts with a value of 0. The "count" facts never
+ // have a value of 0, and the "points" facts aren't meaningful if
+ // they have a value of 0.
+ if ($spec_value == 0) {
+ continue;
+ }
+
$datapoint = $this->getFact($spec_key)
->newDatapoint();
@@ -401,4 +407,5 @@
return $datapoints;
}
+
}
diff --git a/src/applications/fact/engine/PhabricatorTransactionFactEngine.php b/src/applications/fact/engine/PhabricatorTransactionFactEngine.php
new file mode 100644
--- /dev/null
+++ b/src/applications/fact/engine/PhabricatorTransactionFactEngine.php
@@ -0,0 +1,84 @@
+<?php
+
+abstract class PhabricatorTransactionFactEngine
+ extends PhabricatorFactEngine {
+
+ public function newTransactionGroupsForObject(PhabricatorLiskDAO $object) {
+ $viewer = $this->getViewer();
+
+ $xaction_query = PhabricatorApplicationTransactionQuery::newQueryForObject(
+ $object);
+ $xactions = $xaction_query
+ ->setViewer($viewer)
+ ->withObjectPHIDs(array($object->getPHID()))
+ ->execute();
+
+ $xactions = msortv($xactions, 'newChronologicalSortVector');
+
+ return $this->groupTransactions($xactions);
+ }
+
+ protected function groupTransactions(array $xactions) {
+ // These grouping rules are generally much looser than the display grouping
+ // rules. As long as the same user is editing the task and they don't leave
+ // it alone for a particularly long time, we'll group things together.
+
+ $breaks = array();
+
+ $touch_window = phutil_units('15 minutes in seconds');
+ $user_type = PhabricatorPeopleUserPHIDType::TYPECONST;
+
+ $last_actor = null;
+ $last_epoch = null;
+
+ foreach ($xactions as $key => $xaction) {
+ $this_actor = $xaction->getAuthorPHID();
+ if (phid_get_type($this_actor) != $user_type) {
+ $this_actor = null;
+ }
+
+ if ($this_actor && $last_actor && ($this_actor != $last_actor)) {
+ $breaks[$key] = true;
+ }
+
+ // If too much time passed between changes, group them separately.
+ $this_epoch = $xaction->getDateCreated();
+ if ($last_epoch) {
+ if (($this_epoch - $last_epoch) > $touch_window) {
+ $breaks[$key] = true;
+ }
+ }
+
+ // The clock gets reset every time the same real user touches the
+ // task, but does not reset if an automated actor touches things.
+ if (!$last_actor || ($this_actor == $last_actor)) {
+ $last_epoch = $this_epoch;
+ }
+
+ if ($this_actor && ($last_actor != $this_actor)) {
+ $last_actor = $this_actor;
+ $last_epoch = $this_epoch;
+ }
+ }
+
+ $groups = array();
+ $group = array();
+ foreach ($xactions as $key => $xaction) {
+ if (isset($breaks[$key])) {
+ if ($group) {
+ $groups[] = $group;
+ $group = array();
+ }
+ }
+
+ $group[] = $xaction;
+ }
+
+ if ($group) {
+ $groups[] = $group;
+ }
+
+ return $groups;
+ }
+
+}
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Wed, Mar 19, 2:18 PM (5 d, 5 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7394655
Default Alt Text
D19122.id45821.diff (14 KB)
Attached To
Mode
D19122: Fix some of the most obvious bugs in fact generation from Maniphest tasks
Attached
Detach File
Event Timeline
Log In to Comment