1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-22 23:02:42 +01:00

Assign RepositoryIdentity objects to commits

Summary: Depends on D19429. Depends on D19423. Ref T12164. This creates new columns `authorIdentityPHID` and `committerIdentityPHID` on commit objects and starts populating them. Also adds the ability to explicitly set an Identity's assignee to "unassigned()" to null out an incorrect auto-assign. Adds more search functionality to identities. Also creates a daemon task for handling users adding new email address and attempts to associate unclaimed identities.

Test Plan: Imported some repos, watched new columns get populated. Added a new email address for a previous commit, saw daemon job run and assign the identity to the new user. Searched for identities in various and sundry ways.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T12164

Differential Revision: https://secure.phabricator.com/D19443
This commit is contained in:
Austin McKinley 2018-05-09 15:03:49 -07:00
parent f191a66490
commit fe5fde5910
16 changed files with 297 additions and 9 deletions

View file

@ -0,0 +1,3 @@
ALTER TABLE {$NAMESPACE}_repository.repository_commit
ADD COLUMN authorIdentityPHID VARBINARY(64) DEFAULT NULL,
ADD COLUMN committerIdentityPHID VARBINARY(64) DEFAULT NULL;

View file

@ -815,8 +815,12 @@ phutil_register_library_map(array(
'DiffusionHistoryTableView' => 'applications/diffusion/view/DiffusionHistoryTableView.php', 'DiffusionHistoryTableView' => 'applications/diffusion/view/DiffusionHistoryTableView.php',
'DiffusionHistoryView' => 'applications/diffusion/view/DiffusionHistoryView.php', 'DiffusionHistoryView' => 'applications/diffusion/view/DiffusionHistoryView.php',
'DiffusionHovercardEngineExtension' => 'applications/diffusion/engineextension/DiffusionHovercardEngineExtension.php', 'DiffusionHovercardEngineExtension' => 'applications/diffusion/engineextension/DiffusionHovercardEngineExtension.php',
'DiffusionIdentityAssigneeDatasource' => 'applications/diffusion/typeahead/DiffusionIdentityAssigneeDatasource.php',
'DiffusionIdentityAssigneeEditField' => 'applications/diffusion/editfield/DiffusionIdentityAssigneeEditField.php',
'DiffusionIdentityAssigneeSearchField' => 'applications/diffusion/searchfield/DiffusionIdentityAssigneeSearchField.php',
'DiffusionIdentityEditController' => 'applications/diffusion/controller/DiffusionIdentityEditController.php', 'DiffusionIdentityEditController' => 'applications/diffusion/controller/DiffusionIdentityEditController.php',
'DiffusionIdentityListController' => 'applications/diffusion/controller/DiffusionIdentityListController.php', 'DiffusionIdentityListController' => 'applications/diffusion/controller/DiffusionIdentityListController.php',
'DiffusionIdentityUnassignedDatasource' => 'applications/diffusion/typeahead/DiffusionIdentityUnassignedDatasource.php',
'DiffusionIdentityViewController' => 'applications/diffusion/controller/DiffusionIdentityViewController.php', 'DiffusionIdentityViewController' => 'applications/diffusion/controller/DiffusionIdentityViewController.php',
'DiffusionInlineCommentController' => 'applications/diffusion/controller/DiffusionInlineCommentController.php', 'DiffusionInlineCommentController' => 'applications/diffusion/controller/DiffusionInlineCommentController.php',
'DiffusionInlineCommentPreviewController' => 'applications/diffusion/controller/DiffusionInlineCommentPreviewController.php', 'DiffusionInlineCommentPreviewController' => 'applications/diffusion/controller/DiffusionInlineCommentPreviewController.php',
@ -4092,6 +4096,7 @@ phutil_register_library_map(array(
'PhabricatorRepositoryGraphStream' => 'applications/repository/daemon/PhabricatorRepositoryGraphStream.php', 'PhabricatorRepositoryGraphStream' => 'applications/repository/daemon/PhabricatorRepositoryGraphStream.php',
'PhabricatorRepositoryIdentity' => 'applications/repository/storage/PhabricatorRepositoryIdentity.php', 'PhabricatorRepositoryIdentity' => 'applications/repository/storage/PhabricatorRepositoryIdentity.php',
'PhabricatorRepositoryIdentityAssignTransaction' => 'applications/repository/xaction/PhabricatorRepositoryIdentityAssignTransaction.php', 'PhabricatorRepositoryIdentityAssignTransaction' => 'applications/repository/xaction/PhabricatorRepositoryIdentityAssignTransaction.php',
'PhabricatorRepositoryIdentityChangeWorker' => 'applications/repository/worker/PhabricatorRepositoryIdentityChangeWorker.php',
'PhabricatorRepositoryIdentityEditEngine' => 'applications/repository/engine/PhabricatorRepositoryIdentityEditEngine.php', 'PhabricatorRepositoryIdentityEditEngine' => 'applications/repository/engine/PhabricatorRepositoryIdentityEditEngine.php',
'PhabricatorRepositoryIdentityFerretEngine' => 'applications/repository/search/PhabricatorRepositoryIdentityFerretEngine.php', 'PhabricatorRepositoryIdentityFerretEngine' => 'applications/repository/search/PhabricatorRepositoryIdentityFerretEngine.php',
'PhabricatorRepositoryIdentityPHIDType' => 'applications/repository/phid/PhabricatorRepositoryIdentityPHIDType.php', 'PhabricatorRepositoryIdentityPHIDType' => 'applications/repository/phid/PhabricatorRepositoryIdentityPHIDType.php',
@ -6166,8 +6171,12 @@ phutil_register_library_map(array(
'DiffusionHistoryTableView' => 'DiffusionHistoryView', 'DiffusionHistoryTableView' => 'DiffusionHistoryView',
'DiffusionHistoryView' => 'DiffusionView', 'DiffusionHistoryView' => 'DiffusionView',
'DiffusionHovercardEngineExtension' => 'PhabricatorHovercardEngineExtension', 'DiffusionHovercardEngineExtension' => 'PhabricatorHovercardEngineExtension',
'DiffusionIdentityAssigneeDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
'DiffusionIdentityAssigneeEditField' => 'PhabricatorTokenizerEditField',
'DiffusionIdentityAssigneeSearchField' => 'PhabricatorSearchTokenizerField',
'DiffusionIdentityEditController' => 'DiffusionController', 'DiffusionIdentityEditController' => 'DiffusionController',
'DiffusionIdentityListController' => 'DiffusionController', 'DiffusionIdentityListController' => 'DiffusionController',
'DiffusionIdentityUnassignedDatasource' => 'PhabricatorTypeaheadDatasource',
'DiffusionIdentityViewController' => 'DiffusionController', 'DiffusionIdentityViewController' => 'DiffusionController',
'DiffusionInlineCommentController' => 'PhabricatorInlineCommentController', 'DiffusionInlineCommentController' => 'PhabricatorInlineCommentController',
'DiffusionInlineCommentPreviewController' => 'PhabricatorInlineCommentPreviewController', 'DiffusionInlineCommentPreviewController' => 'PhabricatorInlineCommentPreviewController',
@ -10000,6 +10009,7 @@ phutil_register_library_map(array(
'PhabricatorApplicationTransactionInterface', 'PhabricatorApplicationTransactionInterface',
), ),
'PhabricatorRepositoryIdentityAssignTransaction' => 'PhabricatorRepositoryIdentityTransactionType', 'PhabricatorRepositoryIdentityAssignTransaction' => 'PhabricatorRepositoryIdentityTransactionType',
'PhabricatorRepositoryIdentityChangeWorker' => 'PhabricatorWorker',
'PhabricatorRepositoryIdentityEditEngine' => 'PhabricatorEditEngine', 'PhabricatorRepositoryIdentityEditEngine' => 'PhabricatorEditEngine',
'PhabricatorRepositoryIdentityFerretEngine' => 'PhabricatorFerretEngine', 'PhabricatorRepositoryIdentityFerretEngine' => 'PhabricatorFerretEngine',
'PhabricatorRepositoryIdentityPHIDType' => 'PhabricatorPHIDType', 'PhabricatorRepositoryIdentityPHIDType' => 'PhabricatorPHIDType',

View file

@ -104,13 +104,13 @@ final class DiffusionIdentityViewController
} }
$properties->addProperty( $properties->addProperty(
pht('Effective User'), pht('Effective User'),
$viewer->renderHandle($effective_phid)); $this->buildPropertyValue($effective_phid));
$properties->addProperty( $properties->addProperty(
pht('Automatically Detected User'), pht('Automatically Detected User'),
$viewer->renderHandle($automatic_phid)); $this->buildPropertyValue($automatic_phid));
$properties->addProperty( $properties->addProperty(
pht('Manually Set User'), pht('Manually Set User'),
$viewer->renderHandle($manual_phid)); $this->buildPropertyValue($manual_phid));
$header = id(new PHUIHeaderView()) $header = id(new PHUIHeaderView())
->setHeader(array(pht('Identity Assignments'), $tag)); ->setHeader(array(pht('Identity Assignments'), $tag));
@ -120,4 +120,16 @@ final class DiffusionIdentityViewController
->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY)
->addPropertyList($properties); ->addPropertyList($properties);
} }
private function buildPropertyValue($value) {
$viewer = $this->getViewer();
if ($value == DiffusionIdentityUnassignedDatasource::FUNCTION_TOKEN) {
return phutil_tag('em', array(), pht('Explicitly Unassigned'));
} else if (!$value) {
return null;
} else {
return $viewer->renderHandle($value);
}
}
} }

View file

@ -0,0 +1,22 @@
<?php
final class DiffusionIdentityAssigneeEditField
extends PhabricatorTokenizerEditField {
protected function newDatasource() {
return new DiffusionIdentityAssigneeDatasource();
}
protected function newHTTPParameterType() {
return new AphrontUserListHTTPParameterType();
}
protected function newConduitParameterType() {
if ($this->getIsSingleValue()) {
return new ConduitUserParameterType();
} else {
return new ConduitUserListParameterType();
}
}
}

View file

@ -17,6 +17,14 @@ final class DiffusionRepositoryIdentitySearchEngine
protected function buildCustomSearchFields() { protected function buildCustomSearchFields() {
return array( return array(
id(new DiffusionIdentityAssigneeSearchField())
->setLabel(pht('Assigned To'))
->setKey('assignee')
->setDescription(pht('Search for identities by assignee.')),
id(new PhabricatorSearchTextField())
->setLabel(pht('Identity Contains'))
->setKey('match')
->setDescription(pht('Search for identities by substring.')),
id(new PhabricatorSearchThreeStateField()) id(new PhabricatorSearchThreeStateField())
->setLabel(pht('Is Assigned')) ->setLabel(pht('Is Assigned'))
->setKey('hasEffectivePHID') ->setKey('hasEffectivePHID')
@ -34,6 +42,14 @@ final class DiffusionRepositoryIdentitySearchEngine
$query->withHasEffectivePHID($map['hasEffectivePHID']); $query->withHasEffectivePHID($map['hasEffectivePHID']);
} }
if ($map['match'] !== null) {
$query->withIdentityNameLike($map['match']);
}
if ($map['assignee']) {
$query->withAssigneePHIDs($map['assignee']);
}
return $query; return $query;
} }

View file

@ -0,0 +1,22 @@
<?php
final class DiffusionIdentityAssigneeSearchField
extends PhabricatorSearchTokenizerField {
protected function getDefaultValue() {
return array();
}
protected function getValueFromRequest(AphrontRequest $request, $key) {
return $this->getUsersFromRequest($request, $key);
}
protected function newDatasource() {
return new DiffusionIdentityAssigneeDatasource();
}
protected function newConduitParameterType() {
return new ConduitUserListParameterType();
}
}

View file

@ -0,0 +1,21 @@
<?php
final class DiffusionIdentityAssigneeDatasource
extends PhabricatorTypeaheadCompositeDatasource {
public function getBrowseTitle() {
return pht('Browse Assignee');
}
public function getPlaceholderText() {
return pht('Type a username or function...');
}
public function getComponentDatasources() {
return array(
new PhabricatorPeopleDatasource(),
new DiffusionIdentityUnassignedDatasource(),
);
}
}

View file

@ -0,0 +1,77 @@
<?php
final class DiffusionIdentityUnassignedDatasource
extends PhabricatorTypeaheadDatasource {
const FUNCTION_TOKEN = 'unassigned()';
public function getBrowseTitle() {
return pht('Browse Explicitly Unassigned');
}
public function getPlaceholderText() {
return pht('Type "unassigned"...');
}
public function getDatasourceApplicationClass() {
return 'PhabricatorDiffusionApplication';
}
public function getDatasourceFunctions() {
return array(
'unassigned' => array(
'name' => pht('Explicitly Unassigned'),
'summary' => pht('Find results which are not assigned.'),
'description' => pht(
"This function includes results which have been explicitly ".
"unassigned. Use a query like this to find explicitly ".
"unassigned results:\n\n%s\n\n".
"If you combine this function with other functions, the query will ".
"return results which match the other selectors //or// have no ".
"assignee. For example, this query will find results which are ".
"assigned to `alincoln`, and will also find results which have been ".
"unassigned:\n\n%s",
'> unassigned()',
'> alincoln, unassigned()'),
),
);
}
public function loadResults() {
$results = array(
$this->buildUnassignedResult(),
);
return $this->filterResultsAgainstTokens($results);
}
protected function evaluateFunction($function, array $argv_list) {
$results = array();
foreach ($argv_list as $argv) {
$results[] = self::FUNCTION_TOKEN;
}
return $results;
}
public function renderFunctionTokens($function, array $argv_list) {
$results = array();
foreach ($argv_list as $argv) {
$results[] = PhabricatorTypeaheadTokenView::newFromTypeaheadResult(
$this->buildUnassignedResult());
}
return $results;
}
private function buildUnassignedResult() {
$name = pht('Unassigned');
return $this->newFunctionResult()
->setName($name.' unassigned')
->setDisplayName($name)
->setIcon('fa-ban')
->setPHID('unassigned()')
->setUnique(true)
->addAttribute(pht('Select results with no owner.'));
}
}

View file

@ -420,6 +420,12 @@ final class PhabricatorUserEditor extends PhabricatorEditor {
$user->endWriteLocking(); $user->endWriteLocking();
$user->saveTransaction(); $user->saveTransaction();
// Try and match this new address against unclaimed `RepositoryIdentity`s
PhabricatorWorker::scheduleTask(
'PhabricatorRepositoryIdentityChangeWorker',
array('userPHID' => $user->getPHID()),
array('objectPHID' => $user->getPHID()));
return $this; return $this;
} }

View file

@ -75,7 +75,7 @@ final class PhabricatorRepositoryIdentityEditEngine
protected function buildCustomEditFields($object) { protected function buildCustomEditFields($object) {
return array( return array(
id(new PhabricatorUsersEditField()) id(new DiffusionIdentityAssigneeEditField())
->setKey('manuallySetUserPHID') ->setKey('manuallySetUserPHID')
->setLabel(pht('Assigned To')) ->setLabel(pht('Assigned To'))
->setDescription(pht('Override this identity\'s assignment.')) ->setDescription(pht('Override this identity\'s assignment.'))

View file

@ -6,6 +6,9 @@ final class PhabricatorRepositoryIdentityQuery
private $ids; private $ids;
private $phids; private $phids;
private $identityNames; private $identityNames;
private $emailAddress;
private $assigneePHIDs;
private $identityNameLike;
private $hasEffectivePHID; private $hasEffectivePHID;
public function withIDs(array $ids) { public function withIDs(array $ids) {
@ -23,6 +26,21 @@ final class PhabricatorRepositoryIdentityQuery
return $this; return $this;
} }
public function withIdentityNameLike($name_like) {
$this->identityNameLike = $name_like;
return $this;
}
public function withEmailAddress($address) {
$this->emailAddress = $address;
return $this;
}
public function withAssigneePHIDs(array $assignees) {
$this->assigneePHIDs = $assignees;
return $this;
}
public function withHasEffectivePHID($has_effective_phid) { public function withHasEffectivePHID($has_effective_phid) {
$this->hasEffectivePHID = $has_effective_phid; $this->hasEffectivePHID = $has_effective_phid;
return $this; return $this;
@ -57,8 +75,14 @@ final class PhabricatorRepositoryIdentityQuery
$this->phids); $this->phids);
} }
if ($this->hasEffectivePHID !== null) { if ($this->assigneePHIDs !== null) {
$where[] = qsprintf(
$conn,
'repository_identity.currentEffectiveUserPHID IN (%Ls)',
$this->assigneePHIDs);
}
if ($this->hasEffectivePHID !== null) {
if ($this->hasEffectivePHID) { if ($this->hasEffectivePHID) {
$where[] = qsprintf( $where[] = qsprintf(
$conn, $conn,
@ -82,6 +106,21 @@ final class PhabricatorRepositoryIdentityQuery
$name_hashes); $name_hashes);
} }
if ($this->emailAddress !== null) {
$identity_style = "<{$this->emailAddress}>";
$where[] = qsprintf(
$conn,
'repository_identity.identityNameRaw LIKE %<',
$identity_style);
}
if ($this->identityNameLike != null) {
$where[] = qsprintf(
$conn,
'repository_identity.identityNameRaw LIKE %~',
$this->identityNameLike);
}
return $where; return $where;
} }

View file

@ -21,6 +21,8 @@ final class PhabricatorRepositoryCommit
protected $repositoryID; protected $repositoryID;
protected $phid; protected $phid;
protected $authorIdentityPHID;
protected $committerIdentityPHID;
protected $commitIdentifier; protected $commitIdentifier;
protected $epoch; protected $epoch;
protected $mailKey; protected $mailKey;
@ -113,6 +115,8 @@ final class PhabricatorRepositoryCommit
'commitIdentifier' => 'text40', 'commitIdentifier' => 'text40',
'mailKey' => 'bytes20', 'mailKey' => 'bytes20',
'authorPHID' => 'phid?', 'authorPHID' => 'phid?',
'authorIdentityPHID' => 'phid?',
'committerIdentityPHID' => 'phid?',
'auditStatus' => 'uint32', 'auditStatus' => 'uint32',
'summary' => 'text255', 'summary' => 'text255',
'importStatus' => 'uint32', 'importStatus' => 'uint32',

View file

@ -80,6 +80,7 @@ final class PhabricatorRepositoryIdentity
public function getCapabilities() { public function getCapabilities() {
return array( return array(
PhabricatorPolicyCapability::CAN_VIEW, PhabricatorPolicyCapability::CAN_VIEW,
PhabricatorPolicyCapability::CAN_EDIT,
); );
} }

View file

@ -0,0 +1,34 @@
<?php
final class PhabricatorRepositoryIdentityChangeWorker
extends PhabricatorWorker {
protected function doWork() {
$viewer = PhabricatorUser::getOmnipotentUser();
$task_data = $this->getTaskData();
$user_phid = idx($task_data, 'userPHID');
$user = id(new PhabricatorPeopleQuery())
->withPHIDs(array($user_phid))
->setViewer($viewer)
->executeOne();
$emails = id(new PhabricatorUserEmail())->loadAllWhere(
'userPHID = %s ORDER BY address',
$user->getPHID());
foreach ($emails as $email) {
$identities = id(new PhabricatorRepositoryIdentityQuery())
->setViewer($viewer)
->withEmailAddress($email->getAddress())
->execute();
foreach ($identities as $identity) {
$identity->setAutomaticGuessedUserPHID($user->getPHID())
->save();
}
}
}
}

View file

@ -109,6 +109,8 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
$data->setCommitDetail('authorName', $ref->getAuthorName()); $data->setCommitDetail('authorName', $ref->getAuthorName());
$data->setCommitDetail('authorEmail', $ref->getAuthorEmail()); $data->setCommitDetail('authorEmail', $ref->getAuthorEmail());
$data->setCommitDetail(
'authorIdentityPHID', $author_identity->getPHID());
$data->setCommitDetail( $data->setCommitDetail(
'authorPHID', 'authorPHID',
$this->resolveUserPHID($commit, $author)); $this->resolveUserPHID($commit, $author));
@ -124,6 +126,8 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
$data->setCommitDetail( $data->setCommitDetail(
'committerPHID', 'committerPHID',
$this->resolveUserPHID($commit, $committer)); $this->resolveUserPHID($commit, $committer));
$data->setCommitDetail(
'committerIdentityPHID', $committer_identity->getPHID());
} }
$repository = $this->repository; $repository = $this->repository;
@ -161,6 +165,9 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
$commit->setAuthorPHID($author_phid); $commit->setAuthorPHID($author_phid);
} }
$commit->setAuthorIdentityPHID($author_identity->getPHID());
$commit->setCommitterIdentityPHID($committer_identity->getPHID());
$commit->setSummary($data->getSummary()); $commit->setSummary($data->getSummary());
$commit->save(); $commit->save();

View file

@ -21,23 +21,33 @@ final class PhabricatorRepositoryIdentityAssignTransaction
return pht( return pht(
'%s assigned this identity to %s.', '%s assigned this identity to %s.',
$this->renderAuthor(), $this->renderAuthor(),
$this->renderHandle($new)); $this->renderIdentityHandle($new));
} else if (!$new) { } else if (!$new) {
return pht( return pht(
'%s removed %s as the assignee of this identity.', '%s removed %s as the assignee of this identity.',
$this->renderAuthor(), $this->renderAuthor(),
$this->renderHandle($old)); $this->renderIdentityHandle($old));
} else { } else {
return pht( return pht(
'%s changed the assigned user for this identity from %s to %s.', '%s changed the assigned user for this identity from %s to %s.',
$this->renderAuthor(), $this->renderAuthor(),
$this->renderHandle($old), $this->renderIdentityHandle($old),
$this->renderHandle($new)); $this->renderIdentityHandle($new));
}
}
private function renderIdentityHandle($handle) {
$unassigned_token = DiffusionIdentityUnassignedDatasource::FUNCTION_TOKEN;
if ($handle === $unassigned_token) {
return phutil_tag('em', array(), pht('Explicitly Unassigned'));
} else {
return $this->renderHandle($handle);
} }
} }
public function validateTransactions($object, array $xactions) { public function validateTransactions($object, array $xactions) {
$errors = array(); $errors = array();
$unassigned_token = DiffusionIdentityUnassignedDatasource::FUNCTION_TOKEN;
foreach ($xactions as $xaction) { foreach ($xactions as $xaction) {
$old = $xaction->getOldValue(); $old = $xaction->getOldValue();
@ -50,6 +60,10 @@ final class PhabricatorRepositoryIdentityAssignTransaction
continue; continue;
} }
if ($new === $unassigned_token) {
continue;
}
$assignee_list = id(new PhabricatorPeopleQuery()) $assignee_list = id(new PhabricatorPeopleQuery())
->setViewer($this->getActor()) ->setViewer($this->getActor())
->withPHIDs(array($new)) ->withPHIDs(array($new))