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 @@ +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 @@ + 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++;