Uniformized handle data

Summary:
This sets more reasonable values for the object handle fields imo. It's not like I ever want to find out what letter to use and then do `substr($handle->getType(), 0, 1).$handle->getID()` to get `D1` each time I use handles.

Name:

- D1
- T1
- M1
- P1
- etc.

Fullname:

- D1: Something
- T1: Something
- etc.

In addition, this helps me to reasonable prefill Hovercards in case there is no application-specific event listener.

Also deletes `title` and `alternateID` completely. They deserved that.

Test Plan:
Visited places, nothing broke (We only ever used `$handle->getName()` for users and commits).

Tested mail reply handler. Did not test the other way around, but should be fine.

Hovercards broken until D5572 (would love to induce a cyclic dependency)

Reviewers: epriestley, chad, btrahan

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5571
This commit is contained in:
Anh Nhan Nguyen
2013-04-04 13:10:18 -07:00
committed by epriestley
parent 1d4c9b8d8b
commit d3020af97b
5 changed files with 26 additions and 33 deletions

View File

@@ -115,7 +115,7 @@ final class DifferentialManiphestTasksFieldSpecification
$names = array(); $names = array();
foreach ($this->maniphestTasks as $phid) { foreach ($this->maniphestTasks as $phid) {
$handle = $this->getHandle($phid); $handle = $this->getHandle($phid);
$names[] = 'T'.$handle->getAlternateID(); $names[] = $handle->getName();
} }
return implode(', ', $names); return implode(', ', $names);
} }

View File

@@ -285,9 +285,13 @@ EOBODY;
return null; return null;
} }
$user = head(id(new PhabricatorPeopleQuery())
->withPhids(array($handle->getPHID()))
->execute());
$receiver = $this->getMailReceiver(); $receiver = $this->getMailReceiver();
$receiver_id = $receiver->getID(); $receiver_id = $receiver->getID();
$user_id = $handle->getAlternateID(); $user_id = $user->getID();
$hash = PhabricatorMetaMTAReceivedMail::computeMailHash( $hash = PhabricatorMetaMTAReceivedMail::computeMailHash(
$receiver->getMailKey(), $receiver->getMailKey(),
$handle->getPHID()); $handle->getPHID());

View File

@@ -6,13 +6,10 @@ final class PhabricatorObjectHandle {
private $phid; private $phid;
private $type; private $type;
private $name; private $name;
private $email;
private $fullName; private $fullName;
private $imageURI; private $imageURI;
private $timestamp; private $timestamp;
private $alternateID;
private $status = PhabricatorObjectHandleStatus::STATUS_OPEN; private $status = PhabricatorObjectHandleStatus::STATUS_OPEN;
private $title;
private $complete; private $complete;
private $disabled; private $disabled;
@@ -52,11 +49,6 @@ final class PhabricatorObjectHandle {
return $this->status; return $this->status;
} }
public function setTitle($title) {
$this->title = $title;
return $this;
}
public function setFullName($full_name) { public function setFullName($full_name) {
$this->fullName = $full_name; $this->fullName = $full_name;
return $this; return $this;
@@ -96,15 +88,6 @@ final class PhabricatorObjectHandle {
return $this->timestamp; return $this->timestamp;
} }
public function setAlternateID($alternate_id) {
$this->alternateID = $alternate_id;
return $this;
}
public function getAlternateID() {
return $this->alternateID;
}
public function getTypeName() { public function getTypeName() {
static $map = array( static $map = array(
PhabricatorPHIDConstants::PHID_TYPE_USER => 'User', PhabricatorPHIDConstants::PHID_TYPE_USER => 'User',
@@ -190,7 +173,6 @@ final class PhabricatorObjectHandle {
if ($this->status != PhabricatorObjectHandleStatus::STATUS_OPEN) { if ($this->status != PhabricatorObjectHandleStatus::STATUS_OPEN) {
$class .= ' handle-status-'.$this->status; $class .= ' handle-status-'.$this->status;
$title = (isset($this->title) ? $this->title : $this->status);
} }
if ($this->disabled) { if ($this->disabled) {
@@ -203,7 +185,6 @@ final class PhabricatorObjectHandle {
array( array(
'href' => $this->getURI(), 'href' => $this->getURI(),
'class' => $class, 'class' => $class,
'title' => $title,
), ),
$name); $name);
} }

View File

@@ -41,6 +41,7 @@ final class PhabricatorObjectHandleData {
switch ($type) { switch ($type) {
case PhabricatorPHIDConstants::PHID_TYPE_USER: case PhabricatorPHIDConstants::PHID_TYPE_USER:
// TODO: Update query + Batch User Images
$user_dao = new PhabricatorUser(); $user_dao = new PhabricatorUser();
$users = $user_dao->loadAllWhere( $users = $user_dao->loadAllWhere(
'phid in (%Ls)', 'phid in (%Ls)',
@@ -55,6 +56,8 @@ final class PhabricatorObjectHandleData {
return mpull($commits, null, 'getPHID'); return mpull($commits, null, 'getPHID');
case PhabricatorPHIDConstants::PHID_TYPE_TASK: case PhabricatorPHIDConstants::PHID_TYPE_TASK:
// TODO: Update this to ManiphestTaskQuery, //especially// after we have
// policy-awareness
$task_dao = new ManiphestTask(); $task_dao = new ManiphestTask();
$tasks = $task_dao->loadAllWhere( $tasks = $task_dao->loadAllWhere(
'phid IN (%Ls)', 'phid IN (%Ls)',
@@ -69,6 +72,7 @@ final class PhabricatorObjectHandleData {
return mpull($entries, null, 'getPHID'); return mpull($entries, null, 'getPHID');
case PhabricatorPHIDConstants::PHID_TYPE_FILE: case PhabricatorPHIDConstants::PHID_TYPE_FILE:
// TODO: Update this to PhabricatorFileQuery
$object = new PhabricatorFile(); $object = new PhabricatorFile();
$files = $object->loadAllWhere('phid IN (%Ls)', $phids); $files = $object->loadAllWhere('phid IN (%Ls)', $phids);
return mpull($files, null, 'getPHID'); return mpull($files, null, 'getPHID');
@@ -81,6 +85,7 @@ final class PhabricatorObjectHandleData {
return mpull($projects, null, 'getPHID'); return mpull($projects, null, 'getPHID');
case PhabricatorPHIDConstants::PHID_TYPE_REPO: case PhabricatorPHIDConstants::PHID_TYPE_REPO:
// TODO: Update this to PhabricatorRepositoryQuery
$object = new PhabricatorRepository(); $object = new PhabricatorRepository();
$repositories = $object->loadAllWhere('phid in (%Ls)', $phids); $repositories = $object->loadAllWhere('phid in (%Ls)', $phids);
return mpull($repositories, null, 'getPHID'); return mpull($repositories, null, 'getPHID');
@@ -103,6 +108,7 @@ final class PhabricatorObjectHandleData {
return mpull($lists, null, 'getPHID'); return mpull($lists, null, 'getPHID');
case PhabricatorPHIDConstants::PHID_TYPE_DREV: case PhabricatorPHIDConstants::PHID_TYPE_DREV:
// TODO: Update this to DifferentialRevisionQuery
$revision_dao = new DifferentialRevision(); $revision_dao = new DifferentialRevision();
$revisions = $revision_dao->loadAllWhere( $revisions = $revision_dao->loadAllWhere(
'phid IN (%Ls)', 'phid IN (%Ls)',
@@ -110,6 +116,8 @@ final class PhabricatorObjectHandleData {
return mpull($revisions, null, 'getPHID'); return mpull($revisions, null, 'getPHID');
case PhabricatorPHIDConstants::PHID_TYPE_WIKI: case PhabricatorPHIDConstants::PHID_TYPE_WIKI:
// TODO: Update this to PhrictionDocumentQuery, already pre-package
// content DAO
$document_dao = new PhrictionDocument(); $document_dao = new PhrictionDocument();
$documents = $document_dao->loadAllWhere( $documents = $document_dao->loadAllWhere(
'phid IN (%Ls)', 'phid IN (%Ls)',
@@ -260,12 +268,9 @@ final class PhabricatorObjectHandleData {
$handle->setURI('/p/'.$user->getUsername().'/'); $handle->setURI('/p/'.$user->getUsername().'/');
$handle->setFullName( $handle->setFullName(
$user->getUsername().' ('.$user->getRealName().')'); $user->getUsername().' ('.$user->getRealName().')');
$handle->setAlternateID($user->getID());
$handle->setComplete(true); $handle->setComplete(true);
if (isset($statuses[$phid])) { if (isset($statuses[$phid])) {
$handle->setStatus($statuses[$phid]->getTextStatus()); $handle->setStatus($statuses[$phid]->getTextStatus());
$handle->setTitle(
$statuses[$phid]->getTerseSummary($this->viewer));
} }
$handle->setDisabled($user->getIsDisabled()); $handle->setDisabled($user->getIsDisabled());
@@ -308,7 +313,7 @@ final class PhabricatorObjectHandleData {
$handle->setName('Unknown Revision'); $handle->setName('Unknown Revision');
} else { } else {
$rev = $objects[$phid]; $rev = $objects[$phid];
$handle->setName($rev->getTitle()); $handle->setName('D'.$rev->getID());
$handle->setURI('/D'.$rev->getID()); $handle->setURI('/D'.$rev->getID());
$handle->setFullName('D'.$rev->getID().': '.$rev->getTitle()); $handle->setFullName('D'.$rev->getID().': '.$rev->getTitle());
$handle->setComplete(true); $handle->setComplete(true);
@@ -367,11 +372,10 @@ final class PhabricatorObjectHandleData {
$handle->setName('Unknown Task'); $handle->setName('Unknown Task');
} else { } else {
$task = $objects[$phid]; $task = $objects[$phid];
$handle->setName($task->getTitle()); $handle->setName('T'.$task->getID());
$handle->setURI('/T'.$task->getID()); $handle->setURI('/T'.$task->getID());
$handle->setFullName('T'.$task->getID().': '.$task->getTitle()); $handle->setFullName('T'.$task->getID().': '.$task->getTitle());
$handle->setComplete(true); $handle->setComplete(true);
$handle->setAlternateID($task->getID());
if ($task->getStatus() != ManiphestTaskStatus::STATUS_OPEN) { if ($task->getStatus() != ManiphestTaskStatus::STATUS_OPEN) {
$closed = PhabricatorObjectHandleStatus::STATUS_CLOSED; $closed = PhabricatorObjectHandleStatus::STATUS_CLOSED;
$handle->setStatus($closed); $handle->setStatus($closed);
@@ -408,7 +412,8 @@ final class PhabricatorObjectHandleData {
$handle->setName('Unknown File'); $handle->setName('Unknown File');
} else { } else {
$file = $objects[$phid]; $file = $objects[$phid];
$handle->setName($file->getName()); $handle->setName('F'.$file->getID());
$handle->setFullName('F'.$file->getID().' '.$file->getName());
$handle->setURI($file->getBestURI()); $handle->setURI($file->getBestURI());
$handle->setComplete(true); $handle->setComplete(true);
} }
@@ -442,7 +447,7 @@ final class PhabricatorObjectHandleData {
$handle->setName('Unknown Repository'); $handle->setName('Unknown Repository');
} else { } else {
$repository = $objects[$phid]; $repository = $objects[$phid];
$handle->setName($repository->getCallsign()); $handle->setName('r'.$repository->getCallsign());
$handle->setFullName("r" . $repository->getCallsign() . $handle->setFullName("r" . $repository->getCallsign() .
" (" . $repository->getName() . ")"); " (" . $repository->getName() . ")");
$handle->setURI('/diffusion/'.$repository->getCallsign().'/'); $handle->setURI('/diffusion/'.$repository->getCallsign().'/');
@@ -486,6 +491,7 @@ final class PhabricatorObjectHandleData {
break; break;
case PhabricatorPHIDConstants::PHID_TYPE_WIKI: case PhabricatorPHIDConstants::PHID_TYPE_WIKI:
// TODO: Update this
$document_dao = new PhrictionDocument(); $document_dao = new PhrictionDocument();
$content_dao = new PhrictionContent(); $content_dao = new PhrictionContent();
@@ -510,6 +516,7 @@ final class PhabricatorObjectHandleData {
$info = $documents[$phid]; $info = $documents[$phid];
$handle->setName($info['title']); $handle->setName($info['title']);
$handle->setURI(PhrictionDocument::getSlugURI($info['slug'])); $handle->setURI(PhrictionDocument::getSlugURI($info['slug']));
$handle->setFullName($info['title']);
$handle->setComplete(true); $handle->setComplete(true);
if ($info['status'] != PhrictionDocumentStatus::STATUS_EXISTS) { if ($info['status'] != PhrictionDocumentStatus::STATUS_EXISTS) {
$closed = PhabricatorObjectHandleStatus::STATUS_CLOSED; $closed = PhabricatorObjectHandleStatus::STATUS_CLOSED;
@@ -529,7 +536,9 @@ final class PhabricatorObjectHandleData {
$handle->setName('Unknown Ponder Question'); $handle->setName('Unknown Ponder Question');
} else { } else {
$question = $objects[$phid]; $question = $objects[$phid];
$handle->setName(phutil_utf8_shorten($question->getTitle(), 60)); $handle->setName('Q' . $question->getID());
$handle->setFullName(
phutil_utf8_shorten($question->getTitle(), 60));
$handle->setURI(new PhutilURI('/Q' . $question->getID())); $handle->setURI(new PhutilURI('/Q' . $question->getID()));
$handle->setComplete(true); $handle->setComplete(true);
} }
@@ -546,7 +555,7 @@ final class PhabricatorObjectHandleData {
$handle->setName('Unknown Paste'); $handle->setName('Unknown Paste');
} else { } else {
$paste = $objects[$phid]; $paste = $objects[$phid];
$handle->setName($paste->getTitle()); $handle->setName('P'.$paste->getID());
$handle->setFullName($paste->getFullName()); $handle->setFullName($paste->getFullName());
$handle->setURI('/P'.$paste->getID()); $handle->setURI('/P'.$paste->getID());
$handle->setComplete(true); $handle->setComplete(true);
@@ -600,7 +609,7 @@ final class PhabricatorObjectHandleData {
$handle->setName('Unknown Mock'); $handle->setName('Unknown Mock');
} else { } else {
$mock = $objects[$phid]; $mock = $objects[$phid];
$handle->setName($mock->getName()); $handle->setName('M'.$mock->getID());
$handle->setFullName('M'.$mock->getID().': '.$mock->getName()); $handle->setFullName('M'.$mock->getID().': '.$mock->getName());
$handle->setURI('/M'.$mock->getID()); $handle->setURI('/M'.$mock->getID());
$handle->setComplete(true); $handle->setComplete(true);

View File

@@ -22,7 +22,6 @@ abstract class PhabricatorUIExample {
$id = mt_rand(15, 9999); $id = mt_rand(15, 9999);
$handle = new PhabricatorObjectHandle(); $handle = new PhabricatorObjectHandle();
$handle->setAlternateID(mt_rand(15, 9999));
$handle->setName($name); $handle->setName($name);
$handle->setType($type); $handle->setType($type);
$handle->setPHID(PhabricatorPHID::generateNewPHID($type)); $handle->setPHID(PhabricatorPHID::generateNewPHID($type));