Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F13221415
D10348.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
17 KB
Referenced Files
None
Subscribers
None
D10348.diff
View Options
diff --git a/src/applications/diffusion/controller/DiffusionCommitEditController.php b/src/applications/diffusion/controller/DiffusionCommitEditController.php
--- a/src/applications/diffusion/controller/DiffusionCommitEditController.php
+++ b/src/applications/diffusion/controller/DiffusionCommitEditController.php
@@ -8,13 +8,13 @@
}
public function processRequest() {
-
$request = $this->getRequest();
$user = $request->getUser();
$drequest = $this->getDiffusionRequest();
$callsign = $drequest->getRepository()->getCallsign();
$repository = $drequest->getRepository();
$commit = $drequest->loadCommit();
+ $data = $commit->loadCommitData();
$page_title = pht('Edit Diffusion Commit');
if (!$commit) {
@@ -52,7 +52,7 @@
}
$tokenizer_id = celerity_generate_unique_node_id();
- $form = id(new AphrontFormView())
+ $form = id(new AphrontFormView())
->setUser($user)
->setAction($request->getRequestURI()->getPath())
->appendChild(
@@ -72,6 +72,42 @@
pht('Create New Project')))
->setDatasource(new PhabricatorProjectDatasource()));
+ $reason = $data->getCommitDetail('autocloseReason', false);
+ if ($reason !== false) {
+ switch ($reason) {
+ case PhabricatorRepository::BECAUSE_REPOSITORY_IMPORTING:
+ $desc = pht('No, Repository Importing');
+ break;
+ case PhabricatorRepository::BECAUSE_AUTOCLOSE_DISABLED:
+ $desc = pht('No, Autoclose Disabled');
+ break;
+ case PhabricatorRepository::BECAUSE_NOT_ON_AUTOCLOSE_BRANCH:
+ $desc = pht('No, Not On Autoclose Branch');
+ break;
+ case null:
+ $desc = pht('Yes');
+ break;
+ default:
+ $desc = pht('Unknown');
+ break;
+ }
+
+ $doc_href = PhabricatorEnv::getDoclink('Diffusion User Guide: Autoclose');
+ $doc_link = phutil_tag(
+ 'a',
+ array(
+ 'href' => $doc_href,
+ 'target' => '_blank',
+ ),
+ pht('Learn More'));
+
+ $form->appendChild(
+ id(new AphrontFormMarkupControl())
+ ->setLabel(pht('Autoclose?'))
+ ->setValue(array($desc, " \xC2\xB7 ", $doc_link)));
+ }
+
+
Javelin::initBehavior('project-create', array(
'tokenizerID' => $tokenizer_id,
));
@@ -81,12 +117,18 @@
->addCancelButton('/r'.$callsign.$commit->getCommitIdentifier());
$form->appendChild($submit);
+ $crumbs = $this->buildCrumbs(array(
+ 'commit' => true,
+ ));
+ $crumbs->addTextCrumb(pht('Edit'));
+
$form_box = id(new PHUIObjectBoxView())
->setHeaderText($page_title)
->setForm($form);
return $this->buildApplicationPage(
array(
+ $crumbs,
$form_box,
),
array(
diff --git a/src/applications/diffusion/view/DiffusionBranchTableView.php b/src/applications/diffusion/view/DiffusionBranchTableView.php
--- a/src/applications/diffusion/view/DiffusionBranchTableView.php
+++ b/src/applications/diffusion/view/DiffusionBranchTableView.php
@@ -20,6 +20,11 @@
public function render() {
$drequest = $this->getDiffusionRequest();
$current_branch = $drequest->getBranch();
+ $repository = $drequest->getRepository();
+
+ Javelin::initBehavior('phabricator-tooltips');
+
+ $doc_href = PhabricatorEnv::getDoclink('Diffusion User Guide: Autoclose');
$rows = array();
$rowc = array();
@@ -33,6 +38,43 @@
$details = null;
}
+ switch ($repository->shouldSkipAutocloseBranch($branch->getShortName())) {
+ case PhabricatorRepository::BECAUSE_REPOSITORY_IMPORTING:
+ $icon = 'fa-times bluegrey';
+ $tip = pht('Repository Importing');
+ break;
+ case PhabricatorRepository::BECAUSE_AUTOCLOSE_DISABLED:
+ $icon = 'fa-times bluegrey';
+ $tip = pht('Repository Autoclose Disabled');
+ break;
+ case PhabricatorRepository::BECAUSE_BRANCH_UNTRACKED:
+ $icon = 'fa-times bluegrey';
+ $tip = pht('Branch Untracked');
+ break;
+ case PhabricatorRepository::BECAUSE_BRANCH_NOT_AUTOCLOSE:
+ $icon = 'fa-times bluegrey';
+ $tip = pht('Branch Autoclose Disabled');
+ break;
+ case null:
+ $icon = 'fa-check bluegrey';
+ $tip = pht('Autoclose Enabled');
+ break;
+ default:
+ $icon = 'fa-question';
+ $tip = pht('Status Unknown');
+ break;
+ }
+
+ $status_icon = id(new PHUIIconView())
+ ->setIconFont($icon)
+ ->addSigil('has-tooltip')
+ ->setHref($doc_href)
+ ->setMetadata(
+ array(
+ 'tip' => $tip,
+ 'size' => 200,
+ ));
+
$rows[] = array(
phutil_tag(
'a',
@@ -57,6 +99,7 @@
self::linkCommit(
$drequest->getRepository(),
$branch->getCommitIdentifier()),
+ $status_icon,
$datetime,
AphrontTableView::renderSingleDisplayLine($details),
);
@@ -73,6 +116,7 @@
pht('History'),
pht('Branch'),
pht('Head'),
+ pht(''),
pht('Modified'),
pht('Details'),
));
@@ -82,6 +126,7 @@
'pri',
'',
'',
+ '',
'wide',
));
$view->setRowClasses($rowc);
diff --git a/src/applications/herald/adapter/HeraldCommitAdapter.php b/src/applications/herald/adapter/HeraldCommitAdapter.php
--- a/src/applications/herald/adapter/HeraldCommitAdapter.php
+++ b/src/applications/herald/adapter/HeraldCommitAdapter.php
@@ -475,9 +475,7 @@
$refs = DiffusionRepositoryRef::loadAllFromDictionaries($result);
return mpull($refs, 'getShortName');
case self::FIELD_REPOSITORY_AUTOCLOSE_BRANCH:
- return $this->repository->shouldAutocloseCommit(
- $this->commit,
- $this->commitData);
+ return $this->repository->shouldAutocloseCommit($this->commit);
}
return parent::getHeraldField($field);
diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php
--- a/src/applications/repository/storage/PhabricatorRepository.php
+++ b/src/applications/repository/storage/PhabricatorRepository.php
@@ -1,7 +1,8 @@
<?php
/**
- * @task uri Repository URI Management
+ * @task uri Repository URI Management
+ * @task autoclose Autoclose
*/
final class PhabricatorRepository extends PhabricatorRepositoryDAO
implements
@@ -33,6 +34,12 @@
const SERVE_READONLY = 'readonly';
const SERVE_READWRITE = 'readwrite';
+ const BECAUSE_REPOSITORY_IMPORTING = 'auto/importing';
+ const BECAUSE_AUTOCLOSE_DISABLED = 'auto/disabled';
+ const BECAUSE_NOT_ON_AUTOCLOSE_BRANCH = 'auto/nobranch';
+ const BECAUSE_BRANCH_UNTRACKED = 'auto/notrack';
+ const BECAUSE_BRANCH_NOT_AUTOCLOSE = 'auto/noclose';
+
protected $name;
protected $callsign;
protected $uuid;
@@ -586,78 +593,144 @@
return $this->isBranchInFilter($branch, 'branch-filter');
}
- public function shouldAutocloseBranch($branch) {
- if ($this->isImporting()) {
- return false;
+ public function formatCommitName($commit_identifier) {
+ $vcs = $this->getVersionControlSystem();
+
+ $type_git = PhabricatorRepositoryType::REPOSITORY_TYPE_GIT;
+ $type_hg = PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL;
+
+ $is_git = ($vcs == $type_git);
+ $is_hg = ($vcs == $type_hg);
+ if ($is_git || $is_hg) {
+ $short_identifier = substr($commit_identifier, 0, 12);
+ } else {
+ $short_identifier = $commit_identifier;
}
- if ($this->getDetail('disable-autoclose', false)) {
- return false;
+ return 'r'.$this->getCallsign().$short_identifier;
+ }
+
+ public function isImporting() {
+ return (bool)$this->getDetail('importing', false);
+ }
+
+
+/* -( Autoclose )---------------------------------------------------------- */
+
+
+ /**
+ * Determine if autoclose is active for a branch.
+ *
+ * For more details about why, use @{method:shouldSkipAutocloseBranch}.
+ *
+ * @param string Branch name to check.
+ * @return bool True if autoclose is active for the branch.
+ * @task autoclose
+ */
+ public function shouldAutocloseBranch($branch) {
+ return ($this->shouldSkipAutocloseBranch($branch) === null);
+ }
+
+ /**
+ * Determine if autoclose is active for a commit.
+ *
+ * For more details about why, use @{method:shouldSkipAutocloseCommit}.
+ *
+ * @param PhabricatorRepositoryCommit Commit to check.
+ * @return bool True if autoclose is active for the commit.
+ * @task autoclose
+ */
+ public function shouldAutocloseCommit(PhabricatorRepositoryCommit $commit) {
+ return ($this->shouldSkipAutocloseCommit($commit) === null);
+ }
+
+
+ /**
+ * Determine why autoclose should be skipped for a branch.
+ *
+ * This method gives a detailed reason why autoclose will be skipped. To
+ * perform a simple test, use @{method:shouldAutocloseBranch}.
+ *
+ * @param string Branch name to check.
+ * @return const|null Constant identifying reason to skip this branch, or null
+ * if autoclose is active.
+ * @task autoclose
+ */
+ public function shouldSkipAutocloseBranch($branch) {
+ $all_reason = $this->shouldSkipAllAutoclose();
+ if ($all_reason) {
+ return $all_reason;
}
if (!$this->shouldTrackBranch($branch)) {
- return false;
+ return self::BECAUSE_BRANCH_UNTRACKED;
+ }
+
+ if (!$this->isBranchInFilter($branch, 'close-commits-filter')) {
+ return self::BECAUSE_BRANCH_NOT_AUTOCLOSE;
}
- return $this->isBranchInFilter($branch, 'close-commits-filter');
+ return null;
}
- public function shouldAutocloseCommit(
- PhabricatorRepositoryCommit $commit,
- PhabricatorRepositoryCommitData $data) {
- if ($this->getDetail('disable-autoclose', false)) {
- return false;
+ /**
+ * Determine why autoclose should be skipped for a commit.
+ *
+ * This method gives a detailed reason why autoclose will be skipped. To
+ * perform a simple test, use @{method:shouldAutocloseCommit}.
+ *
+ * @param PhabricatorRepositoryCommit Commit to check.
+ * @return const|null Constant identifying reason to skip this commit, or null
+ * if autoclose is active.
+ * @task autoclose
+ */
+ public function shouldSkipAutocloseCommit(
+ PhabricatorRepositoryCommit $commit) {
+
+ $all_reason = $this->shouldSkipAllAutoclose();
+ if ($all_reason) {
+ return $all_reason;
}
switch ($this->getVersionControlSystem()) {
case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
- return true;
+ case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
+ return null;
case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
break;
- case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
- return true;
default:
throw new Exception('Unrecognized version control system.');
}
$closeable_flag = PhabricatorRepositoryCommit::IMPORTED_CLOSEABLE;
- if ($commit->isPartiallyImported($closeable_flag)) {
- return true;
+ if (!$commit->isPartiallyImported($closeable_flag)) {
+ return self::BECAUSE_NOT_ON_AUTOCLOSE_BRANCH;
}
- // TODO: Remove this eventually, it's no longer written to by the import
- // pipeline (however, old tasks may still be queued which don't reflect
- // the new data format).
- $branches = $data->getCommitDetail('seenOnBranches', array());
- foreach ($branches as $branch) {
- if ($this->shouldAutocloseBranch($branch)) {
- return true;
- }
- }
-
- return false;
+ return null;
}
- public function formatCommitName($commit_identifier) {
- $vcs = $this->getVersionControlSystem();
-
- $type_git = PhabricatorRepositoryType::REPOSITORY_TYPE_GIT;
- $type_hg = PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL;
- $is_git = ($vcs == $type_git);
- $is_hg = ($vcs == $type_hg);
- if ($is_git || $is_hg) {
- $short_identifier = substr($commit_identifier, 0, 12);
- } else {
- $short_identifier = $commit_identifier;
+ /**
+ * Determine why all autoclose operations should be skipped for this
+ * repository.
+ *
+ * @return const|null Constant identifying reason to skip all autoclose
+ * operations, or null if autoclose operations are not blocked at the
+ * repository level.
+ * @task autoclose
+ */
+ private function shouldSkipAllAutoclose() {
+ if ($this->isImporting()) {
+ return self::BECAUSE_REPOSITORY_IMPORTING;
}
- return 'r'.$this->getCallsign().$short_identifier;
- }
+ if ($this->getDetail('disable-autoclose', false)) {
+ return self::BECAUSE_AUTOCLOSE_DISABLED;
+ }
- public function isImporting() {
- return (bool)$this->getDetail('importing', false);
+ return null;
}
diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
--- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
+++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
@@ -68,6 +68,16 @@
$commit->setSummary($data->getSummary());
$commit->save();
+ // Figure out if we're going to try to "autoclose" related objects (e.g.,
+ // close linked tasks and related revisions) and, if not, record why we
+ // aren't. Autoclose can be disabled for various reasons at the repository
+ // or commit levels.
+
+ $autoclose_reason = $repository->shouldSkipAutocloseCommit($commit);
+ $data->setCommitDetail('autocloseReason', $autoclose_reason);
+ $should_autoclose = $repository->shouldAutocloseCommit($commit);
+
+
// When updating related objects, we'll act under an omnipotent user to
// ensure we can see them, but take actions as either the committer or
// author (if we recognize their accounts) or the Diffusion application
@@ -90,8 +100,6 @@
// someone probably did something very silly, though.)
$revision = null;
- $should_autoclose = $repository->shouldAutocloseCommit($commit, $data);
-
if ($revision_id) {
$revision_query = id(new DifferentialRevisionQuery())
->withIDs(array($revision_id))
diff --git a/src/docs/user/userguide/diffusion_autoclose.diviner b/src/docs/user/userguide/diffusion_autoclose.diviner
new file mode 100644
--- /dev/null
+++ b/src/docs/user/userguide/diffusion_autoclose.diviner
@@ -0,0 +1,60 @@
+@title Diffusion User Guide: Autoclose
+@group userguide
+
+Explains when Diffusion will close tasks and revisions upon discovery of related
+commits.
+
+Overview
+========
+
+Diffusion can close tasks and revisions when related commits appear in a
+repository. For example, if you make a commit with `Fixes T123` in the commit
+message, Diffusion will close the task `T123`.
+
+This document explains how autoclose works, how to configure it, and how to
+troubleshoot it.
+
+Troubleshooting Autoclose
+=========================
+
+You can check if a branch is currently configured to autoclose on the main
+repository view, or in the branches list view. Hover over the {icon check} or
+{icon times} icon and you should see one of these messages:
+
+ - {icon check} **Autoclose Enabled** Autoclose is active for this branch.
+ - {icon times} **Repository Importing** This repository is still importing.
+ Autoclose does not activate until a repository finishes importing for the
+ first time. This prevents situations where you import a repository and
+ accidentally close hundreds of related objects during import. Autoclose
+ will activate for new commits after the initial import completes.
+ - {icon times} **Repository Autoclose Disabled** Autoclose is disabled for
+ this entire repository. You can enable it in **Edit Repository**.
+ - {icon times} **Branch Untracked** This branch is not tracked. Because it
+ is not tracked, commits on it won't be seen and won't be discovered.
+ - {icon times} **Branch Autoclose Disabled** Autoclose is not enabled for
+ this branch. You can adjust which branches autoclose in **Edit Repository**.
+ This option is only available in Git.
+
+If a branch is in good shape, you can check a specific commit by viewing it
+in the web UI and clicking **Edit Commit**. There should be an **Autoclose?**
+field visible in the form, with possible values listed below.
+
+Note that this field records the state of the world at the time the commit was
+processed, and does not necessarily reflect the current state of the world.
+For example, if a commit did not trigger autoclose because it was processed
+during initial import, the field will still show **No, Repository Importing**
+even after import completes. This means that the commit did not trigger
+autoclose because the repository was importing at the time it was processed,
+not necessarily that the repository is still importing.
+
+ - **Yes** At the time the commit was imported, autoclose triggered and
+ Phabricator attempted to close related objects.
+ - **No, Repository Importing** At the time the commit was processed, the
+ repository was still importing. Autoclose does not activate until a
+ repository fully imports for the first time.
+ - **No, Autoclose Disabled** At the time the commit was processed, the
+ repository had autoclose disabled.
+ - **No, Not On Autoclose Branch** At the time the commit was processed,
+ no containing branch was configured to autoclose.
+ - //Field Not Present// This commit was processed before we implemented
+ this diagnostic feature, and no information is available.
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Mon, May 20, 2:45 AM (3 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6284051
Default Alt Text
D10348.diff (17 KB)
Attached To
Mode
D10348: Improve documentation and tooling around autoclose
Attached
Detach File
Event Timeline
Log In to Comment