Page MenuHomePhabricator

D20308.diff
No OneTemporary

D20308.diff

diff --git a/resources/sql/autopatches/20190322.triggers.01.usage.sql b/resources/sql/autopatches/20190322.triggers.01.usage.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20190322.triggers.01.usage.sql
@@ -0,0 +1,8 @@
+CREATE TABLE {$NAMESPACE}_project.project_triggerusage (
+ id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
+ triggerPHID VARBINARY(64) NOT NULL,
+ examplePHID VARBINARY(64),
+ columnCount INT UNSIGNED NOT NULL,
+ activeColumnCount INT UNSIGNED NOT NULL,
+ UNIQUE KEY `key_trigger` (triggerPHID)
+) ENGINE=InnoDB DEFAULT CHARSET={$CHARSET} COLLATE {$COLLATE_TEXT};
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
@@ -4193,6 +4193,8 @@
'PhabricatorProjectTriggerTransactionQuery' => 'applications/project/query/PhabricatorProjectTriggerTransactionQuery.php',
'PhabricatorProjectTriggerTransactionType' => 'applications/project/xaction/trigger/PhabricatorProjectTriggerTransactionType.php',
'PhabricatorProjectTriggerUnknownRule' => 'applications/project/trigger/PhabricatorProjectTriggerUnknownRule.php',
+ 'PhabricatorProjectTriggerUsage' => 'applications/project/storage/PhabricatorProjectTriggerUsage.php',
+ 'PhabricatorProjectTriggerUsageIndexEngineExtension' => 'applications/project/engineextension/PhabricatorProjectTriggerUsageIndexEngineExtension.php',
'PhabricatorProjectTriggerViewController' => 'applications/project/controller/trigger/PhabricatorProjectTriggerViewController.php',
'PhabricatorProjectTypeTransaction' => 'applications/project/xaction/PhabricatorProjectTypeTransaction.php',
'PhabricatorProjectUIEventListener' => 'applications/project/events/PhabricatorProjectUIEventListener.php',
@@ -10307,6 +10309,7 @@
'PhabricatorProjectDAO',
'PhabricatorApplicationTransactionInterface',
'PhabricatorPolicyInterface',
+ 'PhabricatorIndexableInterface',
'PhabricatorDestructibleInterface',
),
'PhabricatorProjectTriggerController' => 'PhabricatorProjectController',
@@ -10328,6 +10331,8 @@
'PhabricatorProjectTriggerTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
'PhabricatorProjectTriggerTransactionType' => 'PhabricatorModularTransactionType',
'PhabricatorProjectTriggerUnknownRule' => 'PhabricatorProjectTriggerRule',
+ 'PhabricatorProjectTriggerUsage' => 'PhabricatorProjectDAO',
+ 'PhabricatorProjectTriggerUsageIndexEngineExtension' => 'PhabricatorIndexEngineExtension',
'PhabricatorProjectTriggerViewController' => 'PhabricatorProjectTriggerController',
'PhabricatorProjectTypeTransaction' => 'PhabricatorProjectTransactionType',
'PhabricatorProjectUIEventListener' => 'PhabricatorEventListener',
diff --git a/src/applications/project/controller/trigger/PhabricatorProjectTriggerViewController.php b/src/applications/project/controller/trigger/PhabricatorProjectTriggerViewController.php
--- a/src/applications/project/controller/trigger/PhabricatorProjectTriggerViewController.php
+++ b/src/applications/project/controller/trigger/PhabricatorProjectTriggerViewController.php
@@ -71,17 +71,23 @@
// so we load only the columns they can actually see, but have a list of
// all the impacted column PHIDs.
+ // (We're also exposing the status of columns the user might not be able
+ // to see. This technically violates policy, but the trigger usage table
+ // hints at it anyway and it seems unlikely to ever have any security
+ // impact, but is useful in assessing whether a trigger is really in use
+ // or not.)
+
$omnipotent_viewer = PhabricatorUser::getOmnipotentUser();
$all_columns = id(new PhabricatorProjectColumnQuery())
->setViewer($omnipotent_viewer)
->withTriggerPHIDs(array($trigger->getPHID()))
->execute();
- $column_phids = mpull($all_columns, 'getPHID');
+ $column_map = mpull($all_columns, 'getStatus', 'getPHID');
- if ($column_phids) {
+ if ($column_map) {
$visible_columns = id(new PhabricatorProjectColumnQuery())
->setViewer($viewer)
- ->withPHIDs($column_phids)
+ ->withPHIDs(array_keys($column_map))
->execute();
$visible_columns = mpull($visible_columns, null, 'getPHID');
} else {
@@ -89,7 +95,7 @@
}
$rows = array();
- foreach ($column_phids as $column_phid) {
+ foreach ($column_map as $column_phid => $column_status) {
$column = idx($visible_columns, $column_phid);
if ($column) {
@@ -113,7 +119,18 @@
$column_name = phutil_tag('em', array(), pht('Restricted Column'));
}
+ if ($column_status == PhabricatorProjectColumn::STATUS_ACTIVE) {
+ $status_icon = id(new PHUIIconView())
+ ->setIcon('fa-columns', 'blue')
+ ->setTooltip(pht('Active Column'));
+ } else {
+ $status_icon = id(new PHUIIconView())
+ ->setIcon('fa-eye-slash', 'grey')
+ ->setTooltip(pht('Hidden Column'));
+ }
+
$rows[] = array(
+ $status_icon,
$project_name,
$column_name,
);
@@ -123,11 +140,13 @@
->setNoDataString(pht('This trigger is not used by any columns.'))
->setHeaders(
array(
+ null,
pht('Project'),
pht('Column'),
))
->setColumnClasses(
array(
+ null,
null,
'wide pri',
));
diff --git a/src/applications/project/editor/PhabricatorProjectTriggerEditor.php b/src/applications/project/editor/PhabricatorProjectTriggerEditor.php
--- a/src/applications/project/editor/PhabricatorProjectTriggerEditor.php
+++ b/src/applications/project/editor/PhabricatorProjectTriggerEditor.php
@@ -27,4 +27,8 @@
return $types;
}
+ protected function supportsSearch() {
+ return true;
+ }
+
}
diff --git a/src/applications/project/engineextension/PhabricatorProjectTriggerUsageIndexEngineExtension.php b/src/applications/project/engineextension/PhabricatorProjectTriggerUsageIndexEngineExtension.php
new file mode 100644
--- /dev/null
+++ b/src/applications/project/engineextension/PhabricatorProjectTriggerUsageIndexEngineExtension.php
@@ -0,0 +1,69 @@
+<?php
+
+final class PhabricatorProjectTriggerUsageIndexEngineExtension
+ extends PhabricatorIndexEngineExtension {
+
+ const EXTENSIONKEY = 'trigger.usage';
+
+ public function getExtensionName() {
+ return pht('Trigger Usage');
+ }
+
+ public function shouldIndexObject($object) {
+ if (!($object instanceof PhabricatorProjectTrigger)) {
+ return false;
+ }
+
+ return true;
+ }
+
+ public function indexObject(
+ PhabricatorIndexEngine $engine,
+ $object) {
+
+ $usage_table = new PhabricatorProjectTriggerUsage();
+ $column_table = new PhabricatorProjectColumn();
+
+ $conn_w = $object->establishConnection('w');
+
+ $active_statuses = array(
+ PhabricatorProjectColumn::STATUS_ACTIVE,
+ );
+
+ // Select summary information to populate the usage index. When picking
+ // an "examplePHID", we try to pick an active column.
+ $row = queryfx_one(
+ $conn_w,
+ 'SELECT phid, COUNT(*) N, SUM(IF(status IN (%Ls), 1, 0)) M FROM %R
+ WHERE triggerPHID = %s
+ ORDER BY IF(status IN (%Ls), 1, 0) DESC, id ASC',
+ $active_statuses,
+ $column_table,
+ $object->getPHID(),
+ $active_statuses);
+ if ($row) {
+ $example_phid = $row['phid'];
+ $column_count = $row['N'];
+ $active_count = $row['M'];
+ } else {
+ $example_phid = null;
+ $column_count = 0;
+ $active_count = 0;
+ }
+
+ queryfx(
+ $conn_w,
+ 'INSERT INTO %R (triggerPHID, examplePHID, columnCount, activeColumnCount)
+ VALUES (%s, %ns, %d, %d)
+ ON DUPLICATE KEY UPDATE
+ examplePHID = VALUES(examplePHID),
+ columnCount = VALUES(columnCount),
+ activeColumnCount = VALUES(activeColumnCount)',
+ $usage_table,
+ $object->getPHID(),
+ $example_phid,
+ $column_count,
+ $active_count);
+ }
+
+}
diff --git a/src/applications/project/query/PhabricatorProjectTriggerQuery.php b/src/applications/project/query/PhabricatorProjectTriggerQuery.php
--- a/src/applications/project/query/PhabricatorProjectTriggerQuery.php
+++ b/src/applications/project/query/PhabricatorProjectTriggerQuery.php
@@ -5,6 +5,10 @@
private $ids;
private $phids;
+ private $activeColumnMin;
+ private $activeColumnMax;
+
+ private $needUsage;
public function withIDs(array $ids) {
$this->ids = $ids;
@@ -16,6 +20,17 @@
return $this;
}
+ public function needUsage($need_usage) {
+ $this->needUsage = $need_usage;
+ return $this;
+ }
+
+ public function withActiveColumnCountBetween($min, $max) {
+ $this->activeColumnMin = $min;
+ $this->activeColumnMax = $max;
+ return $this;
+ }
+
public function newResultObject() {
return new PhabricatorProjectTrigger();
}
@@ -30,22 +45,91 @@
if ($this->ids !== null) {
$where[] = qsprintf(
$conn,
- 'id IN (%Ld)',
+ 'trigger.id IN (%Ld)',
$this->ids);
}
if ($this->phids !== null) {
$where[] = qsprintf(
$conn,
- 'phid IN (%Ls)',
+ 'trigger.phid IN (%Ls)',
$this->phids);
}
+ if ($this->activeColumnMin !== null) {
+ $where[] = qsprintf(
+ $conn,
+ 'trigger_usage.activeColumnCount >= %d',
+ $this->activeColumnMin);
+ }
+
+ if ($this->activeColumnMax !== null) {
+ $where[] = qsprintf(
+ $conn,
+ 'trigger_usage.activeColumnCount <= %d',
+ $this->activeColumnMax);
+ }
+
return $where;
}
+ protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) {
+ $joins = parent::buildJoinClauseParts($conn);
+
+ if ($this->shouldJoinUsageTable()) {
+ $joins[] = qsprintf(
+ $conn,
+ 'JOIN %R trigger_usage ON trigger.phid = trigger_usage.triggerPHID',
+ new PhabricatorProjectTriggerUsage());
+ }
+
+ return $joins;
+ }
+
+ private function shouldJoinUsageTable() {
+ if ($this->activeColumnMin !== null) {
+ return true;
+ }
+
+ if ($this->activeColumnMax !== null) {
+ return true;
+ }
+
+ return false;
+ }
+
+ protected function didFilterPage(array $triggers) {
+ if ($this->needUsage) {
+ $usage_map = id(new PhabricatorProjectTriggerUsage())->loadAllWhere(
+ 'triggerPHID IN (%Ls)',
+ mpull($triggers, 'getPHID'));
+ $usage_map = mpull($usage_map, null, 'getTriggerPHID');
+
+ foreach ($triggers as $trigger) {
+ $trigger_phid = $trigger->getPHID();
+
+ $usage = idx($usage_map, $trigger_phid);
+ if (!$usage) {
+ $usage = id(new PhabricatorProjectTriggerUsage())
+ ->setTriggerPHID($trigger_phid)
+ ->setExamplePHID(null)
+ ->setColumnCount(0)
+ ->setActiveColumnCount(0);
+ }
+
+ $trigger->attachUsage($usage);
+ }
+ }
+
+ return $triggers;
+ }
+
public function getQueryApplicationClass() {
return 'PhabricatorProjectApplication';
}
+ protected function getPrimaryTableAlias() {
+ return 'trigger';
+ }
+
}
diff --git a/src/applications/project/query/PhabricatorProjectTriggerSearchEngine.php b/src/applications/project/query/PhabricatorProjectTriggerSearchEngine.php
--- a/src/applications/project/query/PhabricatorProjectTriggerSearchEngine.php
+++ b/src/applications/project/query/PhabricatorProjectTriggerSearchEngine.php
@@ -12,16 +12,33 @@
}
public function newQuery() {
- return new PhabricatorProjectTriggerQuery();
+ return id(new PhabricatorProjectTriggerQuery())
+ ->needUsage(true);
}
protected function buildCustomSearchFields() {
- return array();
+ return array(
+ id(new PhabricatorSearchThreeStateField())
+ ->setLabel(pht('Active'))
+ ->setKey('isActive')
+ ->setOptions(
+ pht('(Show All)'),
+ pht('Show Only Active Triggers'),
+ pht('Show Only Inactive Triggers')),
+ );
}
protected function buildQueryFromParameters(array $map) {
$query = $this->newQuery();
+ if ($map['isActive'] !== null) {
+ if ($map['isActive']) {
+ $query->withActiveColumnCountBetween(1, null);
+ } else {
+ $query->withActiveColumnCountBetween(null, 0);
+ }
+ }
+
return $query;
}
@@ -32,7 +49,8 @@
protected function getBuiltinQueryNames() {
$names = array();
- $names['all'] = pht('All');
+ $names['active'] = pht('Active Triggers');
+ $names['all'] = pht('All Triggers');
return $names;
}
@@ -42,6 +60,8 @@
$query->setQueryKey($query_key);
switch ($query_key) {
+ case 'active':
+ return $query->setParameter('isActive', true);
case 'all':
return $query;
}
@@ -56,13 +76,73 @@
assert_instances_of($triggers, 'PhabricatorProjectTrigger');
$viewer = $this->requireViewer();
+ $example_phids = array();
+ foreach ($triggers as $trigger) {
+ $example_phid = $trigger->getUsage()->getExamplePHID();
+ if ($example_phid) {
+ $example_phids[] = $example_phid;
+ }
+ }
+
+ $handles = $viewer->loadHandles($example_phids);
+
$list = id(new PHUIObjectItemListView())
->setViewer($viewer);
foreach ($triggers as $trigger) {
+ $usage = $trigger->getUsage();
+
+ $column_handle = null;
+ $have_column = false;
+ $example_phid = $usage->getExamplePHID();
+ if ($example_phid) {
+ $column_handle = $handles[$example_phid];
+ if ($column_handle->isComplete()) {
+ if (!$column_handle->getPolicyFiltered()) {
+ $have_column = true;
+ }
+ }
+ }
+
+ $column_count = $usage->getColumnCount();
+ $active_count = $usage->getActiveColumnCount();
+
+ if ($have_column) {
+ if ($active_count > 1) {
+ $usage_description = pht(
+ 'Used on %s and %s other active column(s).',
+ $column_handle->renderLink(),
+ new PhutilNumber($active_count - 1));
+ } else if ($column_count > 1) {
+ $usage_description = pht(
+ 'Used on %s and %s other column(s).',
+ $column_handle->renderLink(),
+ new PhutilNumber($column_count - 1));
+ } else {
+ $usage_description = pht(
+ 'Used on %s.',
+ $column_handle->renderLink());
+ }
+ } else {
+ if ($active_count) {
+ $usage_description = pht(
+ 'Used on %s active column(s).',
+ new PhutilNumber($active_count));
+ } else if ($column_count) {
+ $usage_description = pht(
+ 'Used on %s column(s).',
+ new PhutilNumber($column_count));
+ } else {
+ $usage_description = pht(
+ 'Unused trigger.');
+ }
+ }
+
$item = id(new PHUIObjectItemView())
->setObjectName($trigger->getObjectName())
->setHeader($trigger->getDisplayName())
- ->setHref($trigger->getURI());
+ ->setHref($trigger->getURI())
+ ->addAttribute($usage_description)
+ ->setDisabled(!$active_count);
$list->addItem($item);
}
diff --git a/src/applications/project/storage/PhabricatorProjectTrigger.php b/src/applications/project/storage/PhabricatorProjectTrigger.php
--- a/src/applications/project/storage/PhabricatorProjectTrigger.php
+++ b/src/applications/project/storage/PhabricatorProjectTrigger.php
@@ -5,6 +5,7 @@
implements
PhabricatorApplicationTransactionInterface,
PhabricatorPolicyInterface,
+ PhabricatorIndexableInterface,
PhabricatorDestructibleInterface {
protected $name;
@@ -12,6 +13,7 @@
protected $editPolicy;
private $triggerRules;
+ private $usage = self::ATTACHABLE;
public static function initializeNewTrigger() {
$default_edit = PhabricatorPolicies::POLICY_USER;
@@ -257,6 +259,15 @@
return $sounds;
}
+ public function getUsage() {
+ return $this->assertAttached($this->usage);
+ }
+
+ public function attachUsage(PhabricatorProjectTriggerUsage $usage) {
+ $this->usage = $usage;
+ return $this;
+ }
+
/* -( PhabricatorApplicationTransactionInterface )------------------------- */
@@ -310,6 +321,13 @@
new PhabricatorProjectColumn(),
$this->getPHID());
+ // Remove the usage index row for this trigger, if one exists.
+ queryfx(
+ $conn,
+ 'DELETE FROM %R WHERE triggerPHID = %s',
+ new PhabricatorProjectTriggerUsage(),
+ $this->getPHID());
+
$this->delete();
$this->saveTransaction();
diff --git a/src/applications/project/storage/PhabricatorProjectTriggerUsage.php b/src/applications/project/storage/PhabricatorProjectTriggerUsage.php
new file mode 100644
--- /dev/null
+++ b/src/applications/project/storage/PhabricatorProjectTriggerUsage.php
@@ -0,0 +1,28 @@
+<?php
+
+final class PhabricatorProjectTriggerUsage
+ extends PhabricatorProjectDAO {
+
+ protected $triggerPHID;
+ protected $examplePHID;
+ protected $columnCount;
+ protected $activeColumnCount;
+
+ protected function getConfiguration() {
+ return array(
+ self::CONFIG_TIMESTAMPS => false,
+ self::CONFIG_COLUMN_SCHEMA => array(
+ 'examplePHID' => 'phid?',
+ 'columnCount' => 'uint32',
+ 'activeColumnCount' => 'uint32',
+ ),
+ self::CONFIG_KEY_SCHEMA => array(
+ 'key_trigger' => array(
+ 'columns' => array('triggerPHID'),
+ 'unique' => true,
+ ),
+ ),
+ ) + parent::getConfiguration();
+ }
+
+}
diff --git a/src/applications/project/xaction/column/PhabricatorProjectColumnStatusTransaction.php b/src/applications/project/xaction/column/PhabricatorProjectColumnStatusTransaction.php
--- a/src/applications/project/xaction/column/PhabricatorProjectColumnStatusTransaction.php
+++ b/src/applications/project/xaction/column/PhabricatorProjectColumnStatusTransaction.php
@@ -13,6 +13,15 @@
$object->setStatus($value);
}
+ public function applyExternalEffects($object, $value) {
+ // Update the trigger usage index, which cares about whether columns are
+ // active or not.
+ $trigger_phid = $object->getTriggerPHID();
+ if ($trigger_phid) {
+ PhabricatorSearchWorker::queueDocumentForIndexing($trigger_phid);
+ }
+ }
+
public function getTitle() {
$new = $this->getNewValue();
diff --git a/src/applications/project/xaction/column/PhabricatorProjectColumnTriggerTransaction.php b/src/applications/project/xaction/column/PhabricatorProjectColumnTriggerTransaction.php
--- a/src/applications/project/xaction/column/PhabricatorProjectColumnTriggerTransaction.php
+++ b/src/applications/project/xaction/column/PhabricatorProjectColumnTriggerTransaction.php
@@ -13,6 +13,25 @@
$object->setTriggerPHID($value);
}
+ public function applyExternalEffects($object, $value) {
+ // After we change the trigger attached to a column, update the search
+ // indexes for the old and new triggers so we update the usage index.
+ $old = $this->getOldValue();
+ $new = $this->getNewValue();
+
+ $column_phids = array();
+ if ($old) {
+ $column_phids[] = $old;
+ }
+ if ($new) {
+ $column_phids[] = $new;
+ }
+
+ foreach ($column_phids as $phid) {
+ PhabricatorSearchWorker::queueDocumentForIndexing($phid);
+ }
+ }
+
public function getTitle() {
$old = $this->getOldValue();
$new = $this->getNewValue();
diff --git a/src/applications/search/management/PhabricatorSearchManagementIndexWorkflow.php b/src/applications/search/management/PhabricatorSearchManagementIndexWorkflow.php
--- a/src/applications/search/management/PhabricatorSearchManagementIndexWorkflow.php
+++ b/src/applications/search/management/PhabricatorSearchManagementIndexWorkflow.php
@@ -136,7 +136,13 @@
if ($track_skips) {
$new_versions = $this->loadIndexVersions($phid);
- if ($old_versions !== $new_versions) {
+
+ if (!$old_versions && !$new_versions) {
+ // If the document doesn't use an index version, both the lists
+ // of versions will be empty. We still rebuild the index in this
+ // case.
+ $count_updated++;
+ } else if ($old_versions !== $new_versions) {
$count_updated++;
} else {
$count_skipped++;

File Metadata

Mime Type
text/plain
Expires
Mon, Dec 23, 8:43 PM (14 h, 1 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6922027
Default Alt Text
D20308.diff (20 KB)

Event Timeline