1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-09-19 16:58:48 +02:00

Distinguish between "Assigned" and "Effective" identity PHIDs more clearly and consistently

Summary:
Ref T13444. You can currently explicitly unassign an identity (useful if the matching algorithm is misfiring). However, this populates the "currentEffectiveUserPHID" with the "unassigned()" token, which mostly makes things more difficult.

When an identity is explicitly unassigned, convert that into an explicit `null` in the effective user PHID.

Then, realign "assigned" / "effective" language a bit. Previously, `withAssigneePHIDs(...)` actualy queried effective users, which was misleading. Finally, bulk up the list view a little bit to make testing slightly easier.

Test Plan:
  - Unassigned an identity, ran migration, saw `currentEffectiveUserPHID` become `NULL` for the identity.
  - Unassigned a fresh identity, saw NULL.
  - Queried for various identities under the modified constraints.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13444

Differential Revision: https://secure.phabricator.com/D20908
This commit is contained in:
epriestley 2019-11-13 19:24:48 -08:00
parent df0f5c6cee
commit a2b2c391a1
5 changed files with 100 additions and 17 deletions

View file

@ -0,0 +1,3 @@
UPDATE {$NAMESPACE}_repository.repository_identity
SET currentEffectiveUserPHID = NULL
WHERE currentEffectiveUserPHID = 'unassigned()';

View file

@ -25,6 +25,9 @@ final class DiffusionIdentityViewController
->setHeaderIcon('fa-globe'); ->setHeaderIcon('fa-globe');
$crumbs = $this->buildApplicationCrumbs(); $crumbs = $this->buildApplicationCrumbs();
$crumbs->addTextCrumb(
pht('Identities'),
$this->getApplicationURI('identity/'));
$crumbs->addTextCrumb($identity->getObjectName()); $crumbs->addTextCrumb($identity->getObjectName());
$crumbs->setBorder(true); $crumbs->setBorder(true);

View file

@ -17,21 +17,35 @@ final class DiffusionRepositoryIdentitySearchEngine
protected function buildCustomSearchFields() { protected function buildCustomSearchFields() {
return array( return array(
id(new PhabricatorUsersSearchField())
->setLabel(pht('Matching Users'))
->setKey('effectivePHIDs')
->setAliases(
array(
'effective',
'effectivePHID',
))
->setDescription(pht('Search for identities by effective user.')),
id(new DiffusionIdentityAssigneeSearchField()) id(new DiffusionIdentityAssigneeSearchField())
->setLabel(pht('Assigned To')) ->setLabel(pht('Assigned To'))
->setKey('assignee') ->setKey('assignedPHIDs')
->setDescription(pht('Search for identities by assignee.')), ->setAliases(
array(
'assigned',
'assignedPHID',
))
->setDescription(pht('Search for identities by explicit assignee.')),
id(new PhabricatorSearchTextField()) id(new PhabricatorSearchTextField())
->setLabel(pht('Identity Contains')) ->setLabel(pht('Identity Contains'))
->setKey('match') ->setKey('match')
->setDescription(pht('Search for identities by substring.')), ->setDescription(pht('Search for identities by substring.')),
id(new PhabricatorSearchThreeStateField()) id(new PhabricatorSearchThreeStateField())
->setLabel(pht('Is Assigned')) ->setLabel(pht('Has Matching User'))
->setKey('hasEffectivePHID') ->setKey('hasEffectivePHID')
->setOptions( ->setOptions(
pht('(Show All)'), pht('(Show All)'),
pht('Show Only Assigned Identities'), pht('Show Identities With Matching Users'),
pht('Show Only Unassigned Identities')), pht('Show Identities Without Matching Users')),
); );
} }
@ -46,8 +60,12 @@ final class DiffusionRepositoryIdentitySearchEngine
$query->withIdentityNameLike($map['match']); $query->withIdentityNameLike($map['match']);
} }
if ($map['assignee']) { if ($map['assignedPHIDs']) {
$query->withAssigneePHIDs($map['assignee']); $query->withAssignedPHIDs($map['assignedPHIDs']);
}
if ($map['effectivePHIDs']) {
$query->withEffectivePHIDs($map['effectivePHIDs']);
} }
return $query; return $query;
@ -86,15 +104,54 @@ final class DiffusionRepositoryIdentitySearchEngine
$viewer = $this->requireViewer(); $viewer = $this->requireViewer();
$list = new PHUIObjectItemListView(); $list = id(new PHUIObjectItemListView())
$list->setUser($viewer); ->setViewer($viewer);
$phids = array();
foreach ($identities as $identity) {
$phids[] = $identity->getCurrentEffectiveUserPHID();
$phids[] = $identity->getManuallySetUserPHID();
}
$handles = $viewer->loadHandles($phids);
$unassigned = DiffusionIdentityUnassignedDatasource::FUNCTION_TOKEN;
foreach ($identities as $identity) { foreach ($identities as $identity) {
$item = id(new PHUIObjectItemView()) $item = id(new PHUIObjectItemView())
->setObjectName(pht('Identity %d', $identity->getID())) ->setObjectName($identity->getObjectName())
->setHeader($identity->getIdentityShortName()) ->setHeader($identity->getIdentityShortName())
->setHref($identity->getURI()) ->setHref($identity->getURI())
->setObject($identity); ->setObject($identity);
$status_icon = 'fa-circle-o grey';
$effective_phid = $identity->getCurrentEffectiveUserPHID();
if ($effective_phid) {
$item->addIcon(
'fa-id-badge',
pht('Matches User: %s', $handles[$effective_phid]->getName()));
$status_icon = 'fa-id-badge';
}
$assigned_phid = $identity->getManuallySetUserPHID();
if ($assigned_phid) {
if ($assigned_phid === $unassigned) {
$item->addIcon(
'fa-ban',
pht('Explicitly Unassigned'));
$status_icon = 'fa-ban';
} else {
$item->addIcon(
'fa-user',
pht('Assigned To: %s', $handles[$assigned_phid]->getName()));
$status_icon = 'fa-user';
}
}
$item->setStatusIcon($status_icon);
$list->addItem($item); $list->addItem($item);
} }

View file

@ -7,7 +7,8 @@ final class PhabricatorRepositoryIdentityQuery
private $phids; private $phids;
private $identityNames; private $identityNames;
private $emailAddresses; private $emailAddresses;
private $assigneePHIDs; private $assignedPHIDs;
private $effectivePHIDs;
private $identityNameLike; private $identityNameLike;
private $hasEffectivePHID; private $hasEffectivePHID;
@ -36,8 +37,13 @@ final class PhabricatorRepositoryIdentityQuery
return $this; return $this;
} }
public function withAssigneePHIDs(array $assignees) { public function withAssignedPHIDs(array $assigned) {
$this->assigneePHIDs = $assignees; $this->assignedPHIDs = $assigned;
return $this;
}
public function withEffectivePHIDs(array $effective) {
$this->effectivePHIDs = $effective;
return $this; return $this;
} }
@ -75,11 +81,18 @@ final class PhabricatorRepositoryIdentityQuery
$this->phids); $this->phids);
} }
if ($this->assigneePHIDs !== null) { if ($this->assignedPHIDs !== null) {
$where[] = qsprintf(
$conn,
'repository_identity.manuallySetUserPHID IN (%Ls)',
$this->assignedPHIDs);
}
if ($this->effectivePHIDs !== null) {
$where[] = qsprintf( $where[] = qsprintf(
$conn, $conn,
'repository_identity.currentEffectiveUserPHID IN (%Ls)', 'repository_identity.currentEffectiveUserPHID IN (%Ls)',
$this->assigneePHIDs); $this->effectivePHIDs);
} }
if ($this->hasEffectivePHID !== null) { if ($this->hasEffectivePHID !== null) {

View file

@ -96,11 +96,18 @@ final class PhabricatorRepositoryIdentity
public function save() { public function save() {
if ($this->manuallySetUserPHID) { if ($this->manuallySetUserPHID) {
$this->currentEffectiveUserPHID = $this->manuallySetUserPHID; $unassigned = DiffusionIdentityUnassignedDatasource::FUNCTION_TOKEN;
if ($this->manuallySetUserPHID === $unassigned) {
$effective_phid = null;
} else {
$effective_phid = $this->manuallySetUserPHID;
}
} else { } else {
$this->currentEffectiveUserPHID = $this->automaticGuessedUserPHID; $effective_phid = $this->automaticGuessedUserPHID;
} }
$this->setCurrentEffectiveUserPHID($effective_phid);
$email_address = $this->getIdentityEmailAddress(); $email_address = $this->getIdentityEmailAddress();
// Raw identities are unrestricted binary data, and may consequently // Raw identities are unrestricted binary data, and may consequently