From 358baf1b3a180d3e020f5a61f758482f0cdd36fc Mon Sep 17 00:00:00 2001 From: Alan Huang Date: Fri, 3 Aug 2012 11:41:33 -0700 Subject: [PATCH] Improve the flag.* Conduit methods Summary: - Put the code to generate informational dicts about flags into the base class. - Update flag.delete to accept an object PHID in order to delete the flag on that object, since currently the model is that each object may have at most one flag, and each flag has exactly one object, although the former is not enforced. - Add flag.edit, which creates or updates a flag, optionally with the given color and note. Test Plan: Spend endless hours repeatedly running arc call-conduit and arc flag. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D3141 --- src/__phutil_library_map__.php | 4 + .../method/flag/ConduitAPI_flag_Method.php | 28 +++++++ .../flag/ConduitAPI_flag_delete_Method.php | 35 +++++--- .../flag/ConduitAPI_flag_edit_Method.php | 80 +++++++++++++++++++ .../flag/ConduitAPI_flag_query_Method.php | 20 +---- .../phid/ConduitAPI_phid_lookup_Method.php | 66 +++++++++++++++ .../phid/storage/PhabricatorPHID.php | 38 +++++++++ .../PhabricatorSearchController.php | 32 +------- 8 files changed, 245 insertions(+), 58 deletions(-) create mode 100644 src/applications/conduit/method/flag/ConduitAPI_flag_edit_Method.php create mode 100644 src/applications/conduit/method/phid/ConduitAPI_phid_lookup_Method.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 4b8013417d..05bee37f02 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -146,6 +146,7 @@ phutil_register_library_map(array( 'ConduitAPI_file_upload_Method' => 'applications/conduit/method/file/ConduitAPI_file_upload_Method.php', 'ConduitAPI_flag_Method' => 'applications/conduit/method/flag/ConduitAPI_flag_Method.php', 'ConduitAPI_flag_delete_Method' => 'applications/conduit/method/flag/ConduitAPI_flag_delete_Method.php', + 'ConduitAPI_flag_edit_Method' => 'applications/conduit/method/flag/ConduitAPI_flag_edit_Method.php', 'ConduitAPI_flag_query_Method' => 'applications/conduit/method/flag/ConduitAPI_flag_query_Method.php', 'ConduitAPI_macro_Method' => 'applications/conduit/method/macro/ConduitAPI_macro_Method.php', 'ConduitAPI_macro_query_Method' => 'applications/conduit/method/macro/ConduitAPI_macro_query_Method.php', @@ -163,6 +164,7 @@ phutil_register_library_map(array( 'ConduitAPI_path_getowners_Method' => 'applications/conduit/method/path/ConduitAPI_path_getowners_Method.php', 'ConduitAPI_phid_Method' => 'applications/conduit/method/phid/ConduitAPI_phid_Method.php', 'ConduitAPI_phid_info_Method' => 'applications/conduit/method/phid/ConduitAPI_phid_info_Method.php', + 'ConduitAPI_phid_lookup_Method' => 'applications/conduit/method/phid/ConduitAPI_phid_lookup_Method.php', 'ConduitAPI_phid_query_Method' => 'applications/conduit/method/phid/ConduitAPI_phid_query_Method.php', 'ConduitAPI_phriction_Method' => 'applications/conduit/method/phriction/ConduitAPI_phriction_Method.php', 'ConduitAPI_phriction_edit_Method' => 'applications/conduit/method/phriction/ConduitAPI_phriction_edit_Method.php', @@ -1274,6 +1276,7 @@ phutil_register_library_map(array( 'ConduitAPI_file_upload_Method' => 'ConduitAPIMethod', 'ConduitAPI_flag_Method' => 'ConduitAPIMethod', 'ConduitAPI_flag_delete_Method' => 'ConduitAPI_flag_Method', + 'ConduitAPI_flag_edit_Method' => 'ConduitAPI_flag_Method', 'ConduitAPI_flag_query_Method' => 'ConduitAPI_flag_Method', 'ConduitAPI_macro_Method' => 'ConduitAPIMethod', 'ConduitAPI_macro_query_Method' => 'ConduitAPI_macro_Method', @@ -1291,6 +1294,7 @@ phutil_register_library_map(array( 'ConduitAPI_path_getowners_Method' => 'ConduitAPIMethod', 'ConduitAPI_phid_Method' => 'ConduitAPIMethod', 'ConduitAPI_phid_info_Method' => 'ConduitAPI_phid_Method', + 'ConduitAPI_phid_lookup_Method' => 'ConduitAPI_phid_Method', 'ConduitAPI_phid_query_Method' => 'ConduitAPI_phid_Method', 'ConduitAPI_phriction_Method' => 'ConduitAPIMethod', 'ConduitAPI_phriction_edit_Method' => 'ConduitAPI_phriction_Method', diff --git a/src/applications/conduit/method/flag/ConduitAPI_flag_Method.php b/src/applications/conduit/method/flag/ConduitAPI_flag_Method.php index 06fadcadb2..fffefc289c 100644 --- a/src/applications/conduit/method/flag/ConduitAPI_flag_Method.php +++ b/src/applications/conduit/method/flag/ConduitAPI_flag_Method.php @@ -21,5 +21,33 @@ */ abstract class ConduitAPI_flag_Method extends ConduitAPIMethod { + protected function attachHandleToFlag($flag) { + $flag->attachHandle( + PhabricatorObjectHandleData::loadOneHandle($flag->getObjectPHID()) + ); + } + + protected function buildFlagInfoDictionary($flag) { + $color = $flag->getColor(); + $uri = PhabricatorEnv::getProductionURI($flag->getHandle()->getURI()); + + return array( + 'id' => $flag->getID(), + 'ownerPHID' => $flag->getOwnerPHID(), + 'type' => $flag->getType(), + 'objectPHID' => $flag->getObjectPHID(), + 'reasonPHID' => $flag->getReasonPHID(), + 'color' => $color, + 'colorName' => PhabricatorFlagColor::getColorName($color), + 'note' => $flag->getNote(), + 'handle' => array( + 'uri' => $uri, + 'name' => $flag->getHandle()->getName(), + 'fullname' => $flag->getHandle()->getFullName(), + ), + 'dateCreated' => $flag->getDateCreated(), + 'dateModified' => $flag->getDateModified(), + ); + } } diff --git a/src/applications/conduit/method/flag/ConduitAPI_flag_delete_Method.php b/src/applications/conduit/method/flag/ConduitAPI_flag_delete_Method.php index 610c129ec8..68ba0a9d40 100644 --- a/src/applications/conduit/method/flag/ConduitAPI_flag_delete_Method.php +++ b/src/applications/conduit/method/flag/ConduitAPI_flag_delete_Method.php @@ -27,32 +27,49 @@ final class ConduitAPI_flag_delete_Method extends ConduitAPI_flag_Method { public function defineParamTypes() { return array( - 'id' => 'required id', + 'id' => 'optional id', + 'objectPHID' => 'optional phid', ); } public function defineReturnType() { - return 'void'; + return 'dict | null'; } public function defineErrorTypes() { return array( 'ERR_NOT_FOUND' => 'Bad flag ID.', 'ERR_WRONG_USER' => 'You are not the creator of this flag.', + 'ERR_NEED_PARAM' => 'Must pass an id or an objectPHID.', ); } protected function execute(ConduitAPIRequest $request) { $id = $request->getValue('id'); - $flag = id(new PhabricatorFlag())->load($id); - if (!$flag) { - throw new ConduitException('ERR_NOT_FOUND'); - } - if ($flag->getOwnerPHID() != $request->getUser()->getPHID()) { - throw new ConduitException('ERR_WRONG_USER'); + $object = $request->getValue('objectPHID'); + if ($id) { + $flag = id(new PhabricatorFlag())->load($id); + if (!$flag) { + throw new ConduitException('ERR_NOT_FOUND'); + } + if ($flag->getOwnerPHID() != $request->getUser()->getPHID()) { + throw new ConduitException('ERR_WRONG_USER'); + } + } elseif ($object) { + $flag = id(new PhabricatorFlag())->loadOneWhere( + 'objectPHID = %s AND ownerPHID = %s', + $object, + $request->getUser()->getPHID()); + if (!$flag) { + return null; + } + } else { + throw new ConduitException('ERR_NEED_PARAM'); } + $this->attachHandleToFlag($flag); + $ret = $this->buildFlagInfoDictionary($flag); $flag->delete(); - return; + return $ret; } } diff --git a/src/applications/conduit/method/flag/ConduitAPI_flag_edit_Method.php b/src/applications/conduit/method/flag/ConduitAPI_flag_edit_Method.php new file mode 100644 index 0000000000..5dff3ed098 --- /dev/null +++ b/src/applications/conduit/method/flag/ConduitAPI_flag_edit_Method.php @@ -0,0 +1,80 @@ + 'required phid', + 'color' => 'optional int', + 'note' => 'optional string', + ); + } + + public function defineReturnType() { + return 'dict'; + } + + public function defineErrorTypes() { + return array( + ); + } + + protected function execute(ConduitAPIRequest $request) { + $user = $request->getUser()->getPHID(); + $phid = $request->getValue('objectPHID'); + $new = false; + + $flag = id(new PhabricatorFlag())->loadOneWhere( + 'objectPHID = %s AND ownerPHID = %s', + $phid, + $user); + if ($flag) { + $params = $request->getAllParameters(); + if (isset($params['color'])) { + $flag->setColor($params['color']); + } + if (isset($params['note'])) { + $flag->setNote($params['note']); + } + } else { + $default_color = PhabricatorFlagColor::COLOR_BLUE; + $flag = id(new PhabricatorFlag()) + ->setOwnerPHID($user) + ->setType(phid_get_type($phid)) + ->setObjectPHID($phid) + ->setReasonPHID($user) + ->setColor($request->getValue('color', $default_color)) + ->setNote($request->getValue('note', '')); + $new = true; + } + $this->attachHandleToFlag($flag); + $flag->save(); + $ret = $this->buildFlagInfoDictionary($flag); + $ret['new'] = $new; + return $ret; + } + +} diff --git a/src/applications/conduit/method/flag/ConduitAPI_flag_query_Method.php b/src/applications/conduit/method/flag/ConduitAPI_flag_query_Method.php index fc409d44ea..9368977f1e 100644 --- a/src/applications/conduit/method/flag/ConduitAPI_flag_query_Method.php +++ b/src/applications/conduit/method/flag/ConduitAPI_flag_query_Method.php @@ -73,25 +73,7 @@ final class ConduitAPI_flag_query_Method extends ConduitAPI_flag_Method { $results = array(); foreach ($flags as $flag) { - $color = $flag->getColor(); - $uri = PhabricatorEnv::getProductionURI($flag->getHandle()->getURI()); - - $results[] = array( - 'id' => $flag->getID(), - 'ownerPHID' => $flag->getOwnerPHID(), - 'type' => $flag->getType(), - 'objectPHID' => $flag->getObjectPHID(), - 'reasonPHID' => $flag->getReasonPHID(), - 'color' => $color, - 'colorName' => PhabricatorFlagColor::getColorName($color), - 'note' => $flag->getNote(), - 'handle' => array( - 'uri' => $uri, - 'name' => $flag->getHandle()->getName(), - ), - 'dateCreated' => $flag->getDateCreated(), - 'dateModified' => $flag->getDateModified(), - ); + $results[] = $this->buildFlagInfoDictionary($flag); } return $results; diff --git a/src/applications/conduit/method/phid/ConduitAPI_phid_lookup_Method.php b/src/applications/conduit/method/phid/ConduitAPI_phid_lookup_Method.php new file mode 100644 index 0000000000..9dd02ee47d --- /dev/null +++ b/src/applications/conduit/method/phid/ConduitAPI_phid_lookup_Method.php @@ -0,0 +1,66 @@ + 'required list', + ); + } + + public function defineReturnType() { + return 'nonempty dict'; + } + + public function defineErrorTypes() { + return array(); + } + + protected function execute(ConduitAPIRequest $request) { + $names = $request->getValue('names'); + $phids = array(); + foreach ($names as $name) { + $phid = PhabricatorPHID::fromObjectName($name); + if ($phid) { + $phids[$name] = $phid; + } + } + + $handles = id(new PhabricatorObjectHandleData($phids)) + ->loadHandles(); + $result = array(); + foreach ($phids as $name => $phid) { + if (isset($handles[$phid]) && $handles[$phid]->isComplete()) { + $result[$name] = $this->buildHandleInformationDictionary( + $handles[$phid]); + } + } + + return $result; + } + +} diff --git a/src/applications/phid/storage/PhabricatorPHID.php b/src/applications/phid/storage/PhabricatorPHID.php index f72a2d9331..0db3079666 100644 --- a/src/applications/phid/storage/PhabricatorPHID.php +++ b/src/applications/phid/storage/PhabricatorPHID.php @@ -32,4 +32,42 @@ final class PhabricatorPHID { return 'PHID-'.$type.'-'.$uniq; } + public static function fromObjectName($name) { + $object = null; + $match = null; + if (preg_match('/^PHID-[A-Z]+-.{20}$/', $name)) { + // It's already a PHID! Yay. + return $name; + } + if (preg_match('/^r([A-Z]+)(\S*)$/', $name, $match)) { + $repository = id(new PhabricatorRepository()) + ->loadOneWhere('callsign = %s', $match[1]); + if ($match[2] == '') { + $object = $repository; + } else if ($repository) { + $object = id(new PhabricatorRepositoryCommit())->loadOneWhere( + 'repositoryID = %d AND commitIdentifier = %s', + $repository->getID(), + $match[2]); + if (!$object) { + try { + $object = id(new PhabricatorRepositoryCommit())->loadOneWhere( + 'repositoryID = %d AND commitIdentifier LIKE %>', + $repository->getID(), + $match[2]); + } catch (AphrontQueryCountException $ex) { + // Ambiguous, no jump. + } + } + } + } else if (preg_match('/^d(\d+)$/i', $name, $match)) { + $object = id(new DifferentialRevision())->load($match[1]); + } else if (preg_match('/^t(\d+)$/i', $name, $match)) { + $object = id(new ManiphestTask())->load($match[1]); + } + if ($object) { + return $object->getPHID(); + } + return null; + } } diff --git a/src/applications/search/controller/PhabricatorSearchController.php b/src/applications/search/controller/PhabricatorSearchController.php index 93dc03e84b..76b8b7d9a7 100644 --- a/src/applications/search/controller/PhabricatorSearchController.php +++ b/src/applications/search/controller/PhabricatorSearchController.php @@ -222,37 +222,9 @@ final class PhabricatorSearchController $results = $pager->sliceResults($results); if (!$request->getInt('page')) { - $jump = null; - $query_str = $query->getQuery(); - $match = null; - if (preg_match('/^r([A-Z]+)(\S*)$/', $query_str, $match)) { - $repository = id(new PhabricatorRepository()) - ->loadOneWhere('callsign = %s', $match[1]); - if ($match[2] == '') { - $jump = $repository; - } else if ($repository) { - $jump = id(new PhabricatorRepositoryCommit())->loadOneWhere( - 'repositoryID = %d AND commitIdentifier = %s', - $repository->getID(), - $match[2]); - if (!$jump) { - try { - $jump = id(new PhabricatorRepositoryCommit())->loadOneWhere( - 'repositoryID = %d AND commitIdentifier LIKE %>', - $repository->getID(), - $match[2]); - } catch (AphrontQueryCountException $ex) { - // Ambiguous, no jump. - } - } - } - } else if (preg_match('/^d(\d+)$/i', $query_str, $match)) { - $jump = id(new DifferentialRevision())->load($match[1]); - } else if (preg_match('/^t(\d+)$/i', $query_str, $match)) { - $jump = id(new ManiphestTask())->load($match[1]); - } + $jump = PhabricatorPHID::fromObjectName($query->getQuery()); if ($jump) { - array_unshift($results, $jump->getPHID()); + array_unshift($results, $jump); } }