diff --git a/src/applications/differential/conduit/DifferentialFindConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialFindConduitAPIMethod.php index 92dce067fe..79d3fc5d7d 100644 --- a/src/applications/differential/conduit/DifferentialFindConduitAPIMethod.php +++ b/src/applications/differential/conduit/DifferentialFindConduitAPIMethod.php @@ -1,101 +1,99 @@ 'required '.$this->formatStringConstants($types), 'guids' => 'required nonempty list', ); } protected function defineReturnType() { return 'nonempty list'; } protected function execute(ConduitAPIRequest $request) { $type = $request->getValue('query'); $guids = $request->getValue('guids'); $results = array(); if (!$guids) { return $results; } $query = id(new DifferentialRevisionQuery()) ->setViewer($request->getUser()); switch ($type) { case 'open': $query ->withStatus(DifferentialRevisionQuery::STATUS_OPEN) ->withAuthors($guids); break; case 'committable': $query ->withStatus(DifferentialRevisionQuery::STATUS_ACCEPTED) ->withAuthors($guids); break; case 'revision-ids': $query ->withIDs($guids); break; case 'owned': $query->withAuthors($guids); break; case 'phids': $query ->withPHIDs($guids); break; } $revisions = $query->execute(); foreach ($revisions as $revision) { $diff = $revision->loadActiveDiff(); if (!$diff) { continue; } $id = $revision->getID(); $results[] = array( 'id' => $id, 'phid' => $revision->getPHID(), 'name' => $revision->getTitle(), 'uri' => PhabricatorEnv::getProductionURI('/D'.$id), 'dateCreated' => $revision->getDateCreated(), 'authorPHID' => $revision->getAuthorPHID(), - 'statusName' => - ArcanistDifferentialRevisionStatus::getNameForRevisionStatus( - $revision->getStatus()), + 'statusName' => $revision->getStatusDisplayName(), 'sourcePath' => $diff->getSourcePath(), ); } return $results; } } diff --git a/src/applications/differential/conduit/DifferentialGetRevisionConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialGetRevisionConduitAPIMethod.php index b05efb5c15..9109849292 100644 --- a/src/applications/differential/conduit/DifferentialGetRevisionConduitAPIMethod.php +++ b/src/applications/differential/conduit/DifferentialGetRevisionConduitAPIMethod.php @@ -1,101 +1,99 @@ 'required id', ); } protected function defineReturnType() { return 'nonempty dict'; } protected function defineErrorTypes() { return array( 'ERR_BAD_REVISION' => pht('No such revision exists.'), ); } protected function execute(ConduitAPIRequest $request) { $diff = null; $revision_id = $request->getValue('revision_id'); $revision = id(new DifferentialRevisionQuery()) ->withIDs(array($revision_id)) ->setViewer($request->getUser()) ->needReviewers(true) ->executeOne(); if (!$revision) { throw new ConduitException('ERR_BAD_REVISION'); } $reviewer_phids = $revision->getReviewerPHIDs(); $diffs = id(new DifferentialDiffQuery()) ->setViewer($request->getUser()) ->withRevisionIDs(array($revision_id)) ->needChangesets(true) ->execute(); $diff_dicts = mpull($diffs, 'getDiffDict'); $commit_dicts = array(); $commit_phids = $revision->loadCommitPHIDs(); $handles = id(new PhabricatorHandleQuery()) ->setViewer($request->getUser()) ->withPHIDs($commit_phids) ->execute(); foreach ($commit_phids as $commit_phid) { $commit_dicts[] = array( 'fullname' => $handles[$commit_phid]->getFullName(), 'dateCommitted' => $handles[$commit_phid]->getTimestamp(), ); } $field_data = $this->loadCustomFieldsForRevisions( $request->getUser(), array($revision)); $dict = array( 'id' => $revision->getID(), 'phid' => $revision->getPHID(), 'authorPHID' => $revision->getAuthorPHID(), 'uri' => PhabricatorEnv::getURI('/D'.$revision->getID()), 'title' => $revision->getTitle(), 'status' => $revision->getStatus(), - 'statusName' => - ArcanistDifferentialRevisionStatus::getNameForRevisionStatus( - $revision->getStatus()), + 'statusName' => $revision->getStatusDisplayName(), 'summary' => $revision->getSummary(), 'testPlan' => $revision->getTestPlan(), 'lineCount' => $revision->getLineCount(), 'reviewerPHIDs' => $reviewer_phids, 'diffs' => $diff_dicts, 'commits' => $commit_dicts, 'auxiliary' => idx($field_data, $revision->getPHID(), array()), ); return $dict; } } diff --git a/src/applications/differential/conduit/DifferentialQueryConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialQueryConduitAPIMethod.php index 3b88087a67..1051983324 100644 --- a/src/applications/differential/conduit/DifferentialQueryConduitAPIMethod.php +++ b/src/applications/differential/conduit/DifferentialQueryConduitAPIMethod.php @@ -1,254 +1,252 @@ formatStringConstants($hash_types); $status_types = array( DifferentialRevisionQuery::STATUS_ANY, DifferentialRevisionQuery::STATUS_OPEN, DifferentialRevisionQuery::STATUS_ACCEPTED, DifferentialRevisionQuery::STATUS_CLOSED, ); $status_const = $this->formatStringConstants($status_types); $order_types = array( DifferentialRevisionQuery::ORDER_MODIFIED, DifferentialRevisionQuery::ORDER_CREATED, ); $order_const = $this->formatStringConstants($order_types); return array( 'authors' => 'optional list', 'ccs' => 'optional list', 'reviewers' => 'optional list', 'paths' => 'optional list>', 'commitHashes' => 'optional list>', 'status' => 'optional '.$status_const, 'order' => 'optional '.$order_const, 'limit' => 'optional uint', 'offset' => 'optional uint', 'ids' => 'optional list', 'phids' => 'optional list', 'subscribers' => 'optional list', 'responsibleUsers' => 'optional list', 'branches' => 'optional list', ); } protected function defineReturnType() { return 'list'; } protected function defineErrorTypes() { return array( 'ERR-INVALID-PARAMETER' => pht('Missing or malformed parameter.'), ); } protected function execute(ConduitAPIRequest $request) { $authors = $request->getValue('authors'); $ccs = $request->getValue('ccs'); $reviewers = $request->getValue('reviewers'); $status = $request->getValue('status'); $order = $request->getValue('order'); $path_pairs = $request->getValue('paths'); $commit_hashes = $request->getValue('commitHashes'); $limit = $request->getValue('limit'); $offset = $request->getValue('offset'); $ids = $request->getValue('ids'); $phids = $request->getValue('phids'); $subscribers = $request->getValue('subscribers'); $responsible_users = $request->getValue('responsibleUsers'); $branches = $request->getValue('branches'); $query = id(new DifferentialRevisionQuery()) ->setViewer($request->getUser()); if ($authors) { $query->withAuthors($authors); } if ($ccs) { $query->withCCs($ccs); } if ($reviewers) { $query->withReviewers($reviewers); } if ($path_pairs) { $paths = array(); foreach ($path_pairs as $pair) { list($callsign, $path) = $pair; $paths[] = $path; } $path_map = id(new DiffusionPathIDQuery($paths))->loadPathIDs(); if (count($path_map) != count($paths)) { $unknown_paths = array(); foreach ($paths as $p) { if (!idx($path_map, $p)) { $unknown_paths[] = $p; } } throw id(new ConduitException('ERR-INVALID-PARAMETER')) ->setErrorDescription( pht( 'Unknown paths: %s', implode(', ', $unknown_paths))); } $repos = array(); foreach ($path_pairs as $pair) { list($callsign, $path) = $pair; if (!idx($repos, $callsign)) { $repos[$callsign] = id(new PhabricatorRepositoryQuery()) ->setViewer($request->getUser()) ->withCallsigns(array($callsign)) ->executeOne(); if (!$repos[$callsign]) { throw id(new ConduitException('ERR-INVALID-PARAMETER')) ->setErrorDescription( pht( 'Unknown repo callsign: %s', $callsign)); } } $repo = $repos[$callsign]; $query->withPath($repo->getID(), idx($path_map, $path)); } } if ($commit_hashes) { $hash_types = ArcanistDifferentialRevisionHash::getTypes(); foreach ($commit_hashes as $info) { list($type, $hash) = $info; if (empty($type) || !in_array($type, $hash_types) || empty($hash)) { throw new ConduitException('ERR-INVALID-PARAMETER'); } } $query->withCommitHashes($commit_hashes); } if ($status) { $query->withStatus($status); } if ($order) { $query->setOrder($order); } if ($limit) { $query->setLimit($limit); } if ($offset) { $query->setOffset($offset); } if ($ids) { $query->withIDs($ids); } if ($phids) { $query->withPHIDs($phids); } if ($responsible_users) { $query->withResponsibleUsers($responsible_users); } if ($subscribers) { $query->withCCs($subscribers); } if ($branches) { $query->withBranches($branches); } $query->needReviewers(true); $query->needCommitPHIDs(true); $query->needDiffIDs(true); $query->needActiveDiffs(true); $query->needHashes(true); $revisions = $query->execute(); $field_data = $this->loadCustomFieldsForRevisions( $request->getUser(), $revisions); if ($revisions) { $ccs = id(new PhabricatorSubscribersQuery()) ->withObjectPHIDs(mpull($revisions, 'getPHID')) ->execute(); } else { $ccs = array(); } $results = array(); foreach ($revisions as $revision) { $diff = $revision->getActiveDiff(); if (!$diff) { continue; } $id = $revision->getID(); $phid = $revision->getPHID(); $result = array( 'id' => $id, 'phid' => $phid, 'title' => $revision->getTitle(), 'uri' => PhabricatorEnv::getProductionURI('/D'.$id), 'dateCreated' => $revision->getDateCreated(), 'dateModified' => $revision->getDateModified(), 'authorPHID' => $revision->getAuthorPHID(), 'status' => $revision->getStatus(), - 'statusName' => - ArcanistDifferentialRevisionStatus::getNameForRevisionStatus( - $revision->getStatus()), + 'statusName' => $revision->getStatusDisplayName(), 'properties' => $revision->getProperties(), 'branch' => $diff->getBranch(), 'summary' => $revision->getSummary(), 'testPlan' => $revision->getTestPlan(), 'lineCount' => $revision->getLineCount(), 'activeDiffPHID' => $diff->getPHID(), 'diffs' => $revision->getDiffIDs(), 'commits' => $revision->getCommitPHIDs(), 'reviewers' => $revision->getReviewerPHIDs(), 'ccs' => idx($ccs, $phid, array()), 'hashes' => $revision->getHashes(), 'auxiliary' => idx($field_data, $phid, array()), 'repositoryPHID' => $diff->getRepositoryPHID(), ); // TODO: This is a hacky way to put permissions on this field until we // have first-class support, see T838. if ($revision->getAuthorPHID() == $request->getUser()->getPHID()) { $result['sourcePath'] = $diff->getSourcePath(); } $results[] = $result; } return $results; } } diff --git a/src/applications/differential/customfield/DifferentialBranchField.php b/src/applications/differential/customfield/DifferentialBranchField.php index 16be0ce0c9..2387e3cd3f 100644 --- a/src/applications/differential/customfield/DifferentialBranchField.php +++ b/src/applications/differential/customfield/DifferentialBranchField.php @@ -1,96 +1,97 @@ getFieldName(); } public function renderDiffPropertyViewValue(DifferentialDiff $diff) { return $this->getBranchDescription($diff); } private function getBranchDescription(DifferentialDiff $diff) { $branch = $diff->getBranch(); $bookmark = $diff->getBookmark(); if (strlen($branch) && strlen($bookmark)) { return pht('%s (bookmark) on %s (branch)', $bookmark, $branch); } else if (strlen($bookmark)) { return pht('%s (bookmark)', $bookmark); } else if (strlen($branch)) { $onto = $diff->loadTargetBranch(); if (strlen($onto) && ($onto !== $branch)) { return pht( '%s (branched from %s)', $branch, $onto); } else { return $branch; } } else { return null; } } public function getProTips() { return array( pht( 'In Git and Mercurial, use a branch like "%s" to automatically '. 'associate changes with the corresponding task.', 'T123'), ); } public function shouldAppearInTransactionMail() { return true; } public function updateTransactionMailBody( PhabricatorMetaMTAMailBody $body, PhabricatorApplicationTransactionEditor $editor, array $xactions) { - $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; + $revision = $this->getObject(); // Show the "BRANCH" section only if there's a new diff or the revision // is "Accepted". - if ((!$editor->getDiffUpdateTransaction($xactions)) && - ($this->getObject()->getStatus() != $status_accepted)) { + $is_update = (bool)$editor->getDiffUpdateTransaction($xactions); + $is_accepted = $revision->isAccepted(); + if (!$is_update && !$is_accepted) { return; } - $branch = $this->getBranchDescription($this->getObject()->getActiveDiff()); + $branch = $this->getBranchDescription($revision->getActiveDiff()); if ($branch === null) { return; } $body->addTextSection(pht('BRANCH'), $branch); } } diff --git a/src/applications/differential/customfield/DifferentialReviewersField.php b/src/applications/differential/customfield/DifferentialReviewersField.php index 7633bfb492..f835854e2f 100644 --- a/src/applications/differential/customfield/DifferentialReviewersField.php +++ b/src/applications/differential/customfield/DifferentialReviewersField.php @@ -1,95 +1,94 @@ getReviewers(); } public function shouldAppearInPropertyView() { return true; } public function renderPropertyViewLabel() { return $this->getFieldName(); } public function getRequiredHandlePHIDsForPropertyView() { return mpull($this->getUserReviewers(), 'getReviewerPHID'); } public function renderPropertyViewValue(array $handles) { $reviewers = $this->getUserReviewers(); if (!$reviewers) { return phutil_tag('em', array(), pht('None')); } $view = id(new DifferentialReviewersView()) ->setUser($this->getViewer()) ->setReviewers($reviewers) ->setHandles($handles); $diff = $this->getActiveDiff(); if ($diff) { $view->setActiveDiff($diff); } return $view; } private function getUserReviewers() { $reviewers = array(); foreach ($this->getObject()->getReviewers() as $reviewer) { if ($reviewer->isUser()) { $reviewers[] = $reviewer; } } return $reviewers; } public function getRequiredHandlePHIDsForRevisionHeaderWarnings() { return mpull($this->getValue(), 'getReviewerPHID'); } public function getWarningsForRevisionHeader(array $handles) { $revision = $this->getObject(); - $status_needs_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; - if ($revision->getStatus() != $status_needs_review) { + if (!$revision->isNeedsReview()) { return array(); } foreach ($this->getValue() as $reviewer) { if (!$handles[$reviewer->getReviewerPHID()]->isDisabled()) { return array(); } } $warnings = array(); if ($this->getValue()) { $warnings[] = pht( 'This revision needs review, but all specified reviewers are '. 'disabled or inactive.'); } else { $warnings[] = pht( 'This revision needs review, but there are no reviewers specified.'); } return $warnings; } } diff --git a/src/applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php b/src/applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php index 9583486c98..59223f20da 100644 --- a/src/applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php +++ b/src/applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php @@ -1,90 +1,88 @@ getFeedStory(); $action = $story->getStoryData()->getValue('action'); return ($action == DifferentialAction::ACTION_CREATE); } public function isStoryAboutObjectClosure($object) { $story = $this->getFeedStory(); $action = $story->getStoryData()->getValue('action'); return ($action == DifferentialAction::ACTION_CLOSE) || ($action == DifferentialAction::ACTION_ABANDON); } public function willPublishStory($object) { return id(new DifferentialRevisionQuery()) ->setViewer($this->getViewer()) ->withIDs(array($object->getID())) ->needReviewers(true) ->executeOne(); } public function getOwnerPHID($object) { return $object->getAuthorPHID(); } public function getActiveUserPHIDs($object) { - $status = $object->getStatus(); - if ($status == ArcanistDifferentialRevisionStatus::NEEDS_REVIEW) { + if ($object->isNeedsReview()) { return $object->getReviewerPHIDs(); } else { return array(); } } public function getPassiveUserPHIDs($object) { - $status = $object->getStatus(); - if ($status == ArcanistDifferentialRevisionStatus::NEEDS_REVIEW) { + if ($object->isNeedsReview()) { return array(); } else { return $object->getReviewerPHIDs(); } } public function getCCUserPHIDs($object) { return PhabricatorSubscribersQuery::loadSubscribersForPHID( $object->getPHID()); } public function getObjectTitle($object) { $id = $object->getID(); $title = $object->getTitle(); return "D{$id}: {$title}"; } public function getObjectURI($object) { return PhabricatorEnv::getProductionURI('/D'.$object->getID()); } public function getObjectDescription($object) { return $object->getSummary(); } public function isObjectClosed($object) { return $object->isClosed(); } public function getResponsibilityTitle($object) { $prefix = $this->getTitlePrefix($object); return pht('%s Review Request', $prefix); } private function getTitlePrefix(DifferentialRevision $revision) { $prefix_key = 'metamta.differential.subject-prefix'; return PhabricatorEnv::getEnvConfig($prefix_key); } } diff --git a/src/applications/differential/phid/DifferentialRevisionPHIDType.php b/src/applications/differential/phid/DifferentialRevisionPHIDType.php index 5a6cf701ae..8547ec9e83 100644 --- a/src/applications/differential/phid/DifferentialRevisionPHIDType.php +++ b/src/applications/differential/phid/DifferentialRevisionPHIDType.php @@ -1,92 +1,91 @@ withPHIDs($phids); } public function loadHandles( PhabricatorHandleQuery $query, array $handles, array $objects) { foreach ($handles as $phid => $handle) { $revision = $objects[$phid]; $title = $revision->getTitle(); $status = $revision->getStatus(); $monogram = $revision->getMonogram(); $uri = $revision->getURI(); $handle ->setName($monogram) ->setURI($uri) ->setFullName("{$monogram}: {$title}"); if ($revision->isClosed()) { $handle->setStatus(PhabricatorObjectHandle::STATUS_CLOSED); } $status = $revision->getStatus(); $icon = DifferentialRevisionStatus::getRevisionStatusIcon($status); $color = DifferentialRevisionStatus::getRevisionStatusColor($status); - $name = ArcanistDifferentialRevisionStatus::getNameForRevisionStatus( - $status); + $name = $revision->getStatusDisplayName(); $handle ->setStateIcon($icon) ->setStateColor($color) ->setStateName($name); } } public function canLoadNamedObject($name) { return preg_match('/^D[1-9]\d*$/i', $name); } public function loadNamedObjects( PhabricatorObjectQuery $query, array $names) { $id_map = array(); foreach ($names as $name) { $id = (int)substr($name, 1); $id_map[$id][] = $name; } $objects = id(new DifferentialRevisionQuery()) ->setViewer($query->getViewer()) ->withIDs(array_keys($id_map)) ->execute(); $results = array(); foreach ($objects as $id => $object) { foreach (idx($id_map, $id, array()) as $name) { $results[$name] = $object; } } return $results; } } diff --git a/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php b/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php index 6d83d97a47..3e6daa6317 100644 --- a/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php +++ b/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php @@ -1,261 +1,252 @@ objects = $objects; $phids = $query->getEvaluatedParameter('responsiblePHIDs'); if (!$phids) { throw new Exception( pht( 'You can not bucket results by required action without '. 'specifying "Responsible Users".')); } $phids = array_fuse($phids); // Before continuing, throw away any revisions which responsible users // have explicitly resigned from. // The goal is to allow users to resign from revisions they don't want to // review to get these revisions off their dashboard, even if there are // other project or package reviewers which they have authority over. $this->filterResigned($phids); $groups = array(); $groups[] = $this->newGroup() ->setName(pht('Must Review')) ->setKey(self::KEY_MUSTREVIEW) ->setNoDataString(pht('No revisions are blocked on your review.')) ->setObjects($this->filterMustReview($phids)); $groups[] = $this->newGroup() ->setName(pht('Ready to Review')) ->setKey(self::KEY_SHOULDREVIEW) ->setNoDataString(pht('No revisions are waiting on you to review them.')) ->setObjects($this->filterShouldReview($phids)); $groups[] = $this->newGroup() ->setName(pht('Ready to Land')) ->setNoDataString(pht('No revisions are ready to land.')) ->setObjects($this->filterShouldLand($phids)); $groups[] = $this->newGroup() ->setName(pht('Ready to Update')) ->setNoDataString(pht('No revisions are waiting for updates.')) ->setObjects($this->filterShouldUpdate($phids)); $groups[] = $this->newGroup() ->setName(pht('Waiting on Review')) ->setNoDataString(pht('None of your revisions are waiting on review.')) ->setObjects($this->filterWaitingForReview($phids)); $groups[] = $this->newGroup() ->setName(pht('Waiting on Authors')) ->setNoDataString(pht('No revisions are waiting on author action.')) ->setObjects($this->filterWaitingOnAuthors($phids)); $groups[] = $this->newGroup() ->setName(pht('Waiting on Other Reviewers')) ->setNoDataString(pht('No revisions are waiting for other reviewers.')) ->setObjects($this->filterWaitingOnOtherReviewers($phids)); // Because you can apply these buckets to queries which include revisions // that have been closed, add an "Other" bucket if we still have stuff // that didn't get filtered into any of the previous buckets. if ($this->objects) { $groups[] = $this->newGroup() ->setName(pht('Other Revisions')) ->setObjects($this->objects); } return $groups; } private function filterMustReview(array $phids) { $blocking = array( DifferentialReviewerStatus::STATUS_BLOCKING, DifferentialReviewerStatus::STATUS_REJECTED, DifferentialReviewerStatus::STATUS_REJECTED_OLDER, ); $blocking = array_fuse($blocking); $objects = $this->getRevisionsUnderReview($this->objects, $phids); $results = array(); foreach ($objects as $key => $object) { if (!$this->hasReviewersWithStatus($object, $phids, $blocking)) { continue; } $results[$key] = $object; unset($this->objects[$key]); } return $results; } private function filterShouldReview(array $phids) { $reviewing = array( DifferentialReviewerStatus::STATUS_ADDED, DifferentialReviewerStatus::STATUS_COMMENTED, ); $reviewing = array_fuse($reviewing); $objects = $this->getRevisionsUnderReview($this->objects, $phids); $results = array(); foreach ($objects as $key => $object) { if (!$this->hasReviewersWithStatus($object, $phids, $reviewing)) { continue; } $results[$key] = $object; unset($this->objects[$key]); } return $results; } private function filterShouldLand(array $phids) { - $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; - $objects = $this->getRevisionsAuthored($this->objects, $phids); $results = array(); foreach ($objects as $key => $object) { - if ($object->getStatus() != $status_accepted) { + if (!$object->isAccepted()) { continue; } $results[$key] = $object; unset($this->objects[$key]); } return $results; } private function filterShouldUpdate(array $phids) { $statuses = array( ArcanistDifferentialRevisionStatus::NEEDS_REVISION, ArcanistDifferentialRevisionStatus::CHANGES_PLANNED, ArcanistDifferentialRevisionStatus::IN_PREPARATION, ); $statuses = array_fuse($statuses); $objects = $this->getRevisionsAuthored($this->objects, $phids); $results = array(); foreach ($objects as $key => $object) { if (empty($statuses[$object->getStatus()])) { continue; } $results[$key] = $object; unset($this->objects[$key]); } return $results; } private function filterWaitingForReview(array $phids) { - $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; - $objects = $this->getRevisionsAuthored($this->objects, $phids); $results = array(); foreach ($objects as $key => $object) { - if ($object->getStatus() != $status_review) { + if (!$object->isNeedsReview()) { continue; } $results[$key] = $object; unset($this->objects[$key]); } return $results; } private function filterWaitingOnAuthors(array $phids) { $statuses = array( ArcanistDifferentialRevisionStatus::ACCEPTED, ArcanistDifferentialRevisionStatus::NEEDS_REVISION, ArcanistDifferentialRevisionStatus::CHANGES_PLANNED, ArcanistDifferentialRevisionStatus::IN_PREPARATION, ); $statuses = array_fuse($statuses); $objects = $this->getRevisionsNotAuthored($this->objects, $phids); $results = array(); foreach ($objects as $key => $object) { if (empty($statuses[$object->getStatus()])) { continue; } $results[$key] = $object; unset($this->objects[$key]); } return $results; } private function filterWaitingOnOtherReviewers(array $phids) { - $statuses = array( - ArcanistDifferentialRevisionStatus::NEEDS_REVIEW, - ); - $statuses = array_fuse($statuses); - $objects = $this->getRevisionsNotAuthored($this->objects, $phids); $results = array(); foreach ($objects as $key => $object) { - if (!isset($statuses[$object->getStatus()])) { + if (!$object->isNeedsReview()) { continue; } $results[$key] = $object; unset($this->objects[$key]); } return $results; } private function filterResigned(array $phids) { $resigned = array( DifferentialReviewerStatus::STATUS_RESIGNED, ); $resigned = array_fuse($resigned); $objects = $this->getRevisionsNotAuthored($this->objects, $phids); $results = array(); foreach ($objects as $key => $object) { if (!$this->hasReviewersWithStatus($object, $phids, $resigned)) { continue; } $results[$key] = $object; unset($this->objects[$key]); } return $results; } } diff --git a/src/applications/differential/query/DifferentialRevisionResultBucket.php b/src/applications/differential/query/DifferentialRevisionResultBucket.php index 762e2d97f4..54705649eb 100644 --- a/src/applications/differential/query/DifferentialRevisionResultBucket.php +++ b/src/applications/differential/query/DifferentialRevisionResultBucket.php @@ -1,77 +1,76 @@ setAncestorClass(__CLASS__) ->setUniqueMethod('getResultBucketKey') ->execute(); } protected function getRevisionsUnderReview(array $objects, array $phids) { $results = array(); $objects = $this->getRevisionsNotAuthored($objects, $phids); - $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; foreach ($objects as $key => $object) { - if ($object->getStatus() != $status_review) { + if (!$object->isNeedsReview()) { continue; } $results[$key] = $object; } return $results; } protected function getRevisionsAuthored(array $objects, array $phids) { $results = array(); foreach ($objects as $key => $object) { if (isset($phids[$object->getAuthorPHID()])) { $results[$key] = $object; } } return $results; } protected function getRevisionsNotAuthored(array $objects, array $phids) { $results = array(); foreach ($objects as $key => $object) { if (empty($phids[$object->getAuthorPHID()])) { $results[$key] = $object; } } return $results; } protected function hasReviewersWithStatus( DifferentialRevision $revision, array $phids, array $statuses) { foreach ($revision->getReviewers() as $reviewer) { $reviewer_phid = $reviewer->getReviewerPHID(); if (empty($phids[$reviewer_phid])) { continue; } $status = $reviewer->getReviewerStatus(); if (empty($statuses[$status])) { continue; } return true; } return false; } } diff --git a/src/applications/differential/query/DifferentialRevisionSearchEngine.php b/src/applications/differential/query/DifferentialRevisionSearchEngine.php index 4248119cb9..ef88c0ea1c 100644 --- a/src/applications/differential/query/DifferentialRevisionSearchEngine.php +++ b/src/applications/differential/query/DifferentialRevisionSearchEngine.php @@ -1,289 +1,287 @@ needFlags(true) ->needDrafts(true) ->needReviewers(true); } protected function buildQueryFromParameters(array $map) { $query = $this->newQuery(); if ($map['responsiblePHIDs']) { $query->withResponsibleUsers($map['responsiblePHIDs']); } if ($map['authorPHIDs']) { $query->withAuthors($map['authorPHIDs']); } if ($map['reviewerPHIDs']) { $query->withReviewers($map['reviewerPHIDs']); } if ($map['repositoryPHIDs']) { $query->withRepositoryPHIDs($map['repositoryPHIDs']); } if ($map['status']) { $query->withStatus($map['status']); } return $query; } protected function buildCustomSearchFields() { return array( id(new PhabricatorSearchDatasourceField()) ->setLabel(pht('Responsible Users')) ->setKey('responsiblePHIDs') ->setAliases(array('responsiblePHID', 'responsibles', 'responsible')) ->setDatasource(new DifferentialResponsibleDatasource()) ->setDescription( pht('Find revisions that a given user is responsible for.')), id(new PhabricatorUsersSearchField()) ->setLabel(pht('Authors')) ->setKey('authorPHIDs') ->setAliases(array('author', 'authors', 'authorPHID')) ->setDescription( pht('Find revisions with specific authors.')), id(new PhabricatorSearchDatasourceField()) ->setLabel(pht('Reviewers')) ->setKey('reviewerPHIDs') ->setAliases(array('reviewer', 'reviewers', 'reviewerPHID')) ->setDatasource(new DiffusionAuditorFunctionDatasource()) ->setDescription( pht('Find revisions with specific reviewers.')), id(new PhabricatorSearchDatasourceField()) ->setLabel(pht('Repositories')) ->setKey('repositoryPHIDs') ->setAliases(array('repository', 'repositories', 'repositoryPHID')) ->setDatasource(new DiffusionRepositoryFunctionDatasource()) ->setDescription( pht('Find revisions from specific repositories.')), id(new PhabricatorSearchSelectField()) ->setLabel(pht('Status')) ->setKey('status') ->setOptions($this->getStatusOptions()) ->setDescription( pht('Find revisions with particular statuses.')), ); } protected function getURI($path) { return '/differential/'.$path; } protected function getBuiltinQueryNames() { $names = array(); if ($this->requireViewer()->isLoggedIn()) { $names['active'] = pht('Active Revisions'); $names['authored'] = pht('Authored'); } $names['all'] = pht('All Revisions'); return $names; } public function buildSavedQueryFromBuiltin($query_key) { $query = $this->newSavedQuery(); $query->setQueryKey($query_key); $viewer = $this->requireViewer(); switch ($query_key) { case 'active': $bucket_key = DifferentialRevisionRequiredActionResultBucket::BUCKETKEY; return $query ->setParameter('responsiblePHIDs', array($viewer->getPHID())) ->setParameter('status', DifferentialRevisionQuery::STATUS_OPEN) ->setParameter('bucket', $bucket_key); case 'authored': return $query ->setParameter('authorPHIDs', array($viewer->getPHID())); case 'all': return $query; } return parent::buildSavedQueryFromBuiltin($query_key); } private function getStatusOptions() { return array( DifferentialRevisionQuery::STATUS_ANY => pht('All'), DifferentialRevisionQuery::STATUS_OPEN => pht('Open'), DifferentialRevisionQuery::STATUS_ACCEPTED => pht('Accepted'), DifferentialRevisionQuery::STATUS_NEEDS_REVIEW => pht('Needs Review'), DifferentialRevisionQuery::STATUS_NEEDS_REVISION => pht('Needs Revision'), DifferentialRevisionQuery::STATUS_CLOSED => pht('Closed'), DifferentialRevisionQuery::STATUS_ABANDONED => pht('Abandoned'), ); } protected function renderResultList( array $revisions, PhabricatorSavedQuery $query, array $handles) { assert_instances_of($revisions, 'DifferentialRevision'); $viewer = $this->requireViewer(); $template = id(new DifferentialRevisionListView()) ->setUser($viewer) ->setNoBox($this->isPanelContext()); $bucket = $this->getResultBucket($query); $unlanded = $this->loadUnlandedDependencies($revisions); $views = array(); if ($bucket) { $bucket->setViewer($viewer); try { $groups = $bucket->newResultGroups($query, $revisions); foreach ($groups as $group) { // Don't show groups in Dashboard Panels if ($group->getObjects() || !$this->isPanelContext()) { $views[] = id(clone $template) ->setHeader($group->getName()) ->setNoDataString($group->getNoDataString()) ->setRevisions($group->getObjects()); } } } catch (Exception $ex) { $this->addError($ex->getMessage()); } } else { $views[] = id(clone $template) ->setRevisions($revisions) ->setHandles(array()); } if (!$views) { $views[] = id(new DifferentialRevisionListView()) ->setUser($viewer) ->setNoDataString(pht('No revisions found.')); } $phids = array_mergev(mpull($views, 'getRequiredHandlePHIDs')); if ($phids) { $handles = id(new PhabricatorHandleQuery()) ->setViewer($viewer) ->withPHIDs($phids) ->execute(); } else { $handles = array(); } foreach ($views as $view) { $view->setHandles($handles); $view->setUnlandedDependencies($unlanded); } if (count($views) == 1) { // Reduce this to a PHUIObjectItemListView so we can get the free // support from ApplicationSearch. $list = head($views)->render(); } else { $list = $views; } $result = new PhabricatorApplicationSearchResultView(); $result->setContent($list); return $result; } protected function getNewUserBody() { $create_button = id(new PHUIButtonView()) ->setTag('a') ->setText(pht('Create a Diff')) ->setHref('/differential/diff/create/') ->setColor(PHUIButtonView::GREEN); $icon = $this->getApplication()->getIcon(); $app_name = $this->getApplication()->getName(); $view = id(new PHUIBigInfoView()) ->setIcon($icon) ->setTitle(pht('Welcome to %s', $app_name)) ->setDescription( pht('Pre-commit code review. Revisions that are waiting on your input '. 'will appear here.')) ->addAction($create_button); return $view; } private function loadUnlandedDependencies(array $revisions) { - $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; - $phids = array(); foreach ($revisions as $revision) { - if ($revision->getStatus() != $status_accepted) { + if (!$revision->isAccepted()) { continue; } $phids[] = $revision->getPHID(); } if (!$phids) { return array(); } $query = id(new PhabricatorEdgeQuery()) ->withSourcePHIDs($phids) ->withEdgeTypes( array( DifferentialRevisionDependsOnRevisionEdgeType::EDGECONST, )); $query->execute(); $revision_phids = $query->getDestinationPHIDs(); if (!$revision_phids) { return array(); } $viewer = $this->requireViewer(); $blocking_revisions = id(new DifferentialRevisionQuery()) ->setViewer($viewer) ->withPHIDs($revision_phids) ->withStatus(DifferentialRevisionQuery::STATUS_OPEN) ->execute(); $blocking_revisions = mpull($blocking_revisions, null, 'getPHID'); $result = array(); foreach ($revisions as $revision) { $revision_phid = $revision->getPHID(); $blocking_phids = $query->getDestinationPHIDs(array($revision_phid)); $blocking = array_select_keys($blocking_revisions, $blocking_phids); if ($blocking) { $result[$revision_phid] = $blocking; } } return $result; } } diff --git a/src/applications/differential/search/DifferentialRevisionFulltextEngine.php b/src/applications/differential/search/DifferentialRevisionFulltextEngine.php index 45639edbbe..cdb9f1a86d 100644 --- a/src/applications/differential/search/DifferentialRevisionFulltextEngine.php +++ b/src/applications/differential/search/DifferentialRevisionFulltextEngine.php @@ -1,65 +1,64 @@ setViewer($this->getViewer()) ->withPHIDs(array($object->getPHID())) ->needReviewers(true) ->executeOne(); // TODO: This isn't very clean, but custom fields currently rely on it. $object->attachReviewers($revision->getReviewers()); $document->setDocumentTitle($revision->getTitle()); $document->addRelationship( PhabricatorSearchRelationship::RELATIONSHIP_AUTHOR, $revision->getAuthorPHID(), PhabricatorPeopleUserPHIDType::TYPECONST, $revision->getDateCreated()); $document->addRelationship( $revision->isClosed() ? PhabricatorSearchRelationship::RELATIONSHIP_CLOSED : PhabricatorSearchRelationship::RELATIONSHIP_OPEN, $revision->getPHID(), DifferentialRevisionPHIDType::TYPECONST, PhabricatorTime::getNow()); // If a revision needs review, the owners are the reviewers. Otherwise, the // owner is the author (e.g., accepted, rejected, closed). - $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; - if ($revision->getStatus() == $status_review) { + if ($revision->isNeedsReview()) { $reviewers = $revision->getReviewerPHIDs(); $reviewers = array_fuse($reviewers); if ($reviewers) { foreach ($reviewers as $phid) { $document->addRelationship( PhabricatorSearchRelationship::RELATIONSHIP_OWNER, $phid, PhabricatorPeopleUserPHIDType::TYPECONST, $revision->getDateModified()); // Bogus timestamp. } } else { $document->addRelationship( PhabricatorSearchRelationship::RELATIONSHIP_UNOWNED, $revision->getPHID(), PhabricatorPeopleUserPHIDType::TYPECONST, $revision->getDateModified()); // Bogus timestamp. } } else { $document->addRelationship( PhabricatorSearchRelationship::RELATIONSHIP_OWNER, $revision->getAuthorPHID(), PhabricatorPHIDConstants::PHID_TYPE_VOID, $revision->getDateCreated()); } } } diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index 7ad01c02ca..cd6df4b6e5 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -1,923 +1,928 @@ setViewer($actor) ->withClasses(array('PhabricatorDifferentialApplication')) ->executeOne(); $view_policy = $app->getPolicy( DifferentialDefaultViewCapability::CAPABILITY); return id(new DifferentialRevision()) ->setViewPolicy($view_policy) ->setAuthorPHID($actor->getPHID()) ->attachRepository(null) ->attachActiveDiff(null) ->attachReviewers(array()) ->setStatus(ArcanistDifferentialRevisionStatus::NEEDS_REVIEW); } protected function getConfiguration() { return array( self::CONFIG_AUX_PHID => true, self::CONFIG_SERIALIZATION => array( 'attached' => self::SERIALIZATION_JSON, 'unsubscribed' => self::SERIALIZATION_JSON, 'properties' => self::SERIALIZATION_JSON, ), self::CONFIG_COLUMN_SCHEMA => array( 'title' => 'text255', 'originalTitle' => 'text255', 'status' => 'text32', 'summary' => 'text', 'testPlan' => 'text', 'authorPHID' => 'phid?', 'lastReviewerPHID' => 'phid?', 'lineCount' => 'uint32?', 'mailKey' => 'bytes40', 'branchName' => 'text255?', 'repositoryPHID' => 'phid?', ), self::CONFIG_KEY_SCHEMA => array( 'key_phid' => null, 'phid' => array( 'columns' => array('phid'), 'unique' => true, ), 'authorPHID' => array( 'columns' => array('authorPHID', 'status'), ), 'repositoryPHID' => array( 'columns' => array('repositoryPHID'), ), // If you (or a project you are a member of) is reviewing a significant // fraction of the revisions on an install, the result set of open // revisions may be smaller than the result set of revisions where you // are a reviewer. In these cases, this key is better than keys on the // edge table. 'key_status' => array( 'columns' => array('status', 'phid'), ), ), ) + parent::getConfiguration(); } public function setProperty($key, $value) { $this->properties[$key] = $value; return $this; } public function getProperty($key, $default = null) { return idx($this->properties, $key, $default); } public function hasRevisionProperty($key) { return array_key_exists($key, $this->properties); } public function getMonogram() { $id = $this->getID(); return "D{$id}"; } public function getURI() { return '/'.$this->getMonogram(); } public function setTitle($title) { $this->title = $title; if (!$this->getID()) { $this->originalTitle = $title; } return $this; } public function loadIDsByCommitPHIDs($phids) { if (!$phids) { return array(); } $revision_ids = queryfx_all( $this->establishConnection('r'), 'SELECT * FROM %T WHERE commitPHID IN (%Ls)', self::TABLE_COMMIT, $phids); return ipull($revision_ids, 'revisionID', 'commitPHID'); } public function loadCommitPHIDs() { if (!$this->getID()) { return ($this->commits = array()); } $commits = queryfx_all( $this->establishConnection('r'), 'SELECT commitPHID FROM %T WHERE revisionID = %d', self::TABLE_COMMIT, $this->getID()); $commits = ipull($commits, 'commitPHID'); return ($this->commits = $commits); } public function getCommitPHIDs() { return $this->assertAttached($this->commits); } public function getActiveDiff() { // TODO: Because it's currently technically possible to create a revision // without an associated diff, we allow an attached-but-null active diff. // It would be good to get rid of this once we make diff-attaching // transactional. return $this->assertAttached($this->activeDiff); } public function attachActiveDiff($diff) { $this->activeDiff = $diff; return $this; } public function getDiffIDs() { return $this->assertAttached($this->diffIDs); } public function attachDiffIDs(array $ids) { rsort($ids); $this->diffIDs = array_values($ids); return $this; } public function attachCommitPHIDs(array $phids) { $this->commits = array_values($phids); return $this; } public function getAttachedPHIDs($type) { return array_keys(idx($this->attached, $type, array())); } public function setAttachedPHIDs($type, array $phids) { $this->attached[$type] = array_fill_keys($phids, array()); return $this; } public function generatePHID() { return PhabricatorPHID::generateNewPHID( DifferentialRevisionPHIDType::TYPECONST); } public function loadActiveDiff() { return id(new DifferentialDiff())->loadOneWhere( 'revisionID = %d ORDER BY id DESC LIMIT 1', $this->getID()); } public function save() { if (!$this->getMailKey()) { $this->mailKey = Filesystem::readRandomCharacters(40); } return parent::save(); } public function getHashes() { return $this->assertAttached($this->hashes); } public function attachHashes(array $hashes) { $this->hashes = $hashes; return $this; } public function canReviewerForceAccept( PhabricatorUser $viewer, DifferentialReviewer $reviewer) { if (!$reviewer->isPackage()) { return false; } $map = $this->getReviewerForceAcceptMap($viewer); if (!$map) { return false; } if (isset($map[$reviewer->getReviewerPHID()])) { return true; } return false; } private function getReviewerForceAcceptMap(PhabricatorUser $viewer) { $fragment = $viewer->getCacheFragment(); if (!array_key_exists($fragment, $this->forceMap)) { $map = $this->newReviewerForceAcceptMap($viewer); $this->forceMap[$fragment] = $map; } return $this->forceMap[$fragment]; } private function newReviewerForceAcceptMap(PhabricatorUser $viewer) { $diff = $this->getActiveDiff(); if (!$diff) { return null; } $repository_phid = $diff->getRepositoryPHID(); if (!$repository_phid) { return null; } $paths = array(); try { $changesets = $diff->getChangesets(); } catch (Exception $ex) { $changesets = id(new DifferentialChangesetQuery()) ->setViewer($viewer) ->withDiffs(array($diff)) ->execute(); } foreach ($changesets as $changeset) { $paths[] = $changeset->getOwnersFilename(); } if (!$paths) { return null; } $reviewer_phids = array(); foreach ($this->getReviewers() as $reviewer) { if (!$reviewer->isPackage()) { continue; } $reviewer_phids[] = $reviewer->getReviewerPHID(); } if (!$reviewer_phids) { return null; } // Load all the reviewing packages which have control over some of the // paths in the change. These are packages which the actor may be able // to force-accept on behalf of. $control_query = id(new PhabricatorOwnersPackageQuery()) ->setViewer($viewer) ->withStatuses(array(PhabricatorOwnersPackage::STATUS_ACTIVE)) ->withPHIDs($reviewer_phids) ->withControl($repository_phid, $paths); $control_packages = $control_query->execute(); if (!$control_packages) { return null; } // Load all the packages which have potential control over some of the // paths in the change and are owned by the actor. These are packages // which the actor may be able to use their authority over to gain the // ability to force-accept for other packages. This query doesn't apply // dominion rules yet, and we'll bypass those rules later on. $authority_query = id(new PhabricatorOwnersPackageQuery()) ->setViewer($viewer) ->withStatuses(array(PhabricatorOwnersPackage::STATUS_ACTIVE)) ->withAuthorityPHIDs(array($viewer->getPHID())) ->withControl($repository_phid, $paths); $authority_packages = $authority_query->execute(); if (!$authority_packages) { return null; } $authority_packages = mpull($authority_packages, null, 'getPHID'); // Build a map from each path in the revision to the reviewer packages // which control it. $control_map = array(); foreach ($paths as $path) { $control_packages = $control_query->getControllingPackagesForPath( $repository_phid, $path); // Remove packages which the viewer has authority over. We don't need // to check these for force-accept because they can just accept them // normally. $control_packages = mpull($control_packages, null, 'getPHID'); foreach ($control_packages as $phid => $control_package) { if (isset($authority_packages[$phid])) { unset($control_packages[$phid]); } } if (!$control_packages) { continue; } $control_map[$path] = $control_packages; } if (!$control_map) { return null; } // From here on out, we only care about paths which we have at least one // controlling package for. $paths = array_keys($control_map); // Now, build a map from each path to the packages which would control it // if there were no dominion rules. $authority_map = array(); foreach ($paths as $path) { $authority_packages = $authority_query->getControllingPackagesForPath( $repository_phid, $path, $ignore_dominion = true); $authority_map[$path] = mpull($authority_packages, null, 'getPHID'); } // For each path, find the most general package that the viewer has // authority over. For example, we'll prefer a package that owns "/" to a // package that owns "/src/". $force_map = array(); foreach ($authority_map as $path => $package_map) { $path_fragments = PhabricatorOwnersPackage::splitPath($path); $fragment_count = count($path_fragments); // Find the package that we have authority over which has the most // general match for this path. $best_match = null; $best_package = null; foreach ($package_map as $package_phid => $package) { $package_paths = $package->getPathsForRepository($repository_phid); foreach ($package_paths as $package_path) { // NOTE: A strength of 0 means "no match". A strength of 1 means // that we matched "/", so we can not possibly find another stronger // match. $strength = $package_path->getPathMatchStrength( $path_fragments, $fragment_count); if (!$strength) { continue; } if ($strength < $best_match || !$best_package) { $best_match = $strength; $best_package = $package; if ($strength == 1) { break 2; } } } } if ($best_package) { $force_map[$path] = array( 'strength' => $best_match, 'package' => $best_package, ); } } // For each path which the viewer owns a package for, find other packages // which that authority can be used to force-accept. Once we find a way to // force-accept a package, we don't need to keep loooking. $has_control = array(); foreach ($force_map as $path => $spec) { $path_fragments = PhabricatorOwnersPackage::splitPath($path); $fragment_count = count($path_fragments); $authority_strength = $spec['strength']; $control_packages = $control_map[$path]; foreach ($control_packages as $control_phid => $control_package) { if (isset($has_control[$control_phid])) { continue; } $control_paths = $control_package->getPathsForRepository( $repository_phid); foreach ($control_paths as $control_path) { $strength = $control_path->getPathMatchStrength( $path_fragments, $fragment_count); if (!$strength) { continue; } if ($strength > $authority_strength) { $authority = $spec['package']; $has_control[$control_phid] = array( 'authority' => $authority, 'phid' => $authority->getPHID(), ); break; } } } } // Return a map from packages which may be force accepted to the packages // which permit that forced acceptance. return ipull($has_control, 'phid'); } /* -( PhabricatorPolicyInterface )----------------------------------------- */ public function getCapabilities() { return array( PhabricatorPolicyCapability::CAN_VIEW, PhabricatorPolicyCapability::CAN_EDIT, ); } public function getPolicy($capability) { switch ($capability) { case PhabricatorPolicyCapability::CAN_VIEW: return $this->getViewPolicy(); case PhabricatorPolicyCapability::CAN_EDIT: return $this->getEditPolicy(); } } public function hasAutomaticCapability($capability, PhabricatorUser $user) { // A revision's author (which effectively means "owner" after we added // commandeering) can always view and edit it. $author_phid = $this->getAuthorPHID(); if ($author_phid) { if ($user->getPHID() == $author_phid) { return true; } } return false; } public function describeAutomaticCapability($capability) { $description = array( pht('The owner of a revision can always view and edit it.'), ); switch ($capability) { case PhabricatorPolicyCapability::CAN_VIEW: $description[] = pht( 'If a revision belongs to a repository, other users must be able '. 'to view the repository in order to view the revision.'); break; } return $description; } /* -( PhabricatorExtendedPolicyInterface )--------------------------------- */ public function getExtendedPolicy($capability, PhabricatorUser $viewer) { $extended = array(); switch ($capability) { case PhabricatorPolicyCapability::CAN_VIEW: $repository_phid = $this->getRepositoryPHID(); $repository = $this->getRepository(); // Try to use the object if we have it, since it will save us some // data fetching later on. In some cases, we might not have it. $repository_ref = nonempty($repository, $repository_phid); if ($repository_ref) { $extended[] = array( $repository_ref, PhabricatorPolicyCapability::CAN_VIEW, ); } break; } return $extended; } /* -( PhabricatorTokenReceiverInterface )---------------------------------- */ public function getUsersToNotifyOfTokenGiven() { return array( $this->getAuthorPHID(), ); } public function getReviewers() { return $this->assertAttached($this->reviewerStatus); } public function attachReviewers(array $reviewers) { assert_instances_of($reviewers, 'DifferentialReviewer'); $reviewers = mpull($reviewers, null, 'getReviewerPHID'); $this->reviewerStatus = $reviewers; return $this; } public function getReviewerPHIDs() { $reviewers = $this->getReviewers(); return mpull($reviewers, 'getReviewerPHID'); } public function getReviewerPHIDsForEdit() { $reviewers = $this->getReviewers(); $status_blocking = DifferentialReviewerStatus::STATUS_BLOCKING; $value = array(); foreach ($reviewers as $reviewer) { $phid = $reviewer->getReviewerPHID(); if ($reviewer->getReviewerStatus() == $status_blocking) { $value[] = 'blocking('.$phid.')'; } else { $value[] = $phid; } } return $value; } public function getRepository() { return $this->assertAttached($this->repository); } public function attachRepository(PhabricatorRepository $repository = null) { $this->repository = $repository; return $this; } public function isClosed() { return DifferentialRevisionStatus::isClosedStatus($this->getStatus()); } public function isAbandoned() { $status_abandoned = ArcanistDifferentialRevisionStatus::ABANDONED; return ($this->getStatus() == $status_abandoned); } public function isAccepted() { $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; return ($this->getStatus() == $status_accepted); } + public function isNeedsReview() { + $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; + return ($this->getStatus() == $status_review); + } + public function getStatusIcon() { $map = array( ArcanistDifferentialRevisionStatus::NEEDS_REVIEW => 'fa-code grey', ArcanistDifferentialRevisionStatus::NEEDS_REVISION => 'fa-refresh red', ArcanistDifferentialRevisionStatus::CHANGES_PLANNED => 'fa-headphones red', ArcanistDifferentialRevisionStatus::ACCEPTED => 'fa-check green', ArcanistDifferentialRevisionStatus::CLOSED => 'fa-check-square-o black', ArcanistDifferentialRevisionStatus::ABANDONED => 'fa-plane black', ); return idx($map, $this->getStatus()); } public function getStatusDisplayName() { $status = $this->getStatus(); return ArcanistDifferentialRevisionStatus::getNameForRevisionStatus( $status); } public function getFlag(PhabricatorUser $viewer) { return $this->assertAttachedKey($this->flags, $viewer->getPHID()); } public function attachFlag( PhabricatorUser $viewer, PhabricatorFlag $flag = null) { $this->flags[$viewer->getPHID()] = $flag; return $this; } public function getHasDraft(PhabricatorUser $viewer) { return $this->assertAttachedKey($this->drafts, $viewer->getCacheFragment()); } public function attachHasDraft(PhabricatorUser $viewer, $has_draft) { $this->drafts[$viewer->getCacheFragment()] = $has_draft; return $this; } /* -( HarbormasterBuildableInterface )------------------------------------- */ public function getHarbormasterBuildableDisplayPHID() { return $this->getHarbormasterContainerPHID(); } public function getHarbormasterBuildablePHID() { return $this->loadActiveDiff()->getPHID(); } public function getHarbormasterContainerPHID() { return $this->getPHID(); } public function getHarbormasterPublishablePHID() { return $this->getPHID(); } public function getBuildVariables() { return array(); } public function getAvailableBuildVariables() { return array(); } /* -( PhabricatorSubscribableInterface )----------------------------------- */ public function isAutomaticallySubscribed($phid) { if ($phid == $this->getAuthorPHID()) { return true; } // TODO: This only happens when adding or removing CCs, and is safe from a // policy perspective, but the subscription pathway should have some // opportunity to load this data properly. For now, this is the only case // where implicit subscription is not an intrinsic property of the object. if ($this->reviewerStatus == self::ATTACHABLE) { $reviewers = id(new DifferentialRevisionQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) ->withPHIDs(array($this->getPHID())) ->needReviewers(true) ->executeOne() ->getReviewers(); } else { $reviewers = $this->getReviewers(); } foreach ($reviewers as $reviewer) { if ($reviewer->getReviewerPHID() == $phid) { return true; } } return false; } /* -( PhabricatorCustomFieldInterface )------------------------------------ */ public function getCustomFieldSpecificationForRole($role) { return PhabricatorEnv::getEnvConfig('differential.fields'); } public function getCustomFieldBaseClass() { return 'DifferentialCustomField'; } public function getCustomFields() { return $this->assertAttached($this->customFields); } public function attachCustomFields(PhabricatorCustomFieldAttachment $fields) { $this->customFields = $fields; return $this; } /* -( PhabricatorApplicationTransactionInterface )------------------------- */ public function getApplicationTransactionEditor() { return new DifferentialTransactionEditor(); } public function getApplicationTransactionObject() { return $this; } public function getApplicationTransactionTemplate() { return new DifferentialTransaction(); } public function willRenderTimeline( PhabricatorApplicationTransactionView $timeline, AphrontRequest $request) { $viewer = $request->getViewer(); $render_data = $timeline->getRenderData(); $left = $request->getInt('left', idx($render_data, 'left')); $right = $request->getInt('right', idx($render_data, 'right')); $diffs = id(new DifferentialDiffQuery()) ->setViewer($request->getUser()) ->withIDs(array($left, $right)) ->execute(); $diffs = mpull($diffs, null, 'getID'); $left_diff = $diffs[$left]; $right_diff = $diffs[$right]; $old_ids = $request->getStr('old', idx($render_data, 'old')); $new_ids = $request->getStr('new', idx($render_data, 'new')); $old_ids = array_filter(explode(',', $old_ids)); $new_ids = array_filter(explode(',', $new_ids)); $type_inline = DifferentialTransaction::TYPE_INLINE; $changeset_ids = array_merge($old_ids, $new_ids); $inlines = array(); foreach ($timeline->getTransactions() as $xaction) { if ($xaction->getTransactionType() == $type_inline) { $inlines[] = $xaction->getComment(); $changeset_ids[] = $xaction->getComment()->getChangesetID(); } } if ($changeset_ids) { $changesets = id(new DifferentialChangesetQuery()) ->setViewer($request->getUser()) ->withIDs($changeset_ids) ->execute(); $changesets = mpull($changesets, null, 'getID'); } else { $changesets = array(); } foreach ($inlines as $key => $inline) { $inlines[$key] = DifferentialInlineComment::newFromModernComment( $inline); } $query = id(new DifferentialInlineCommentQuery()) ->needHidden(true) ->setViewer($viewer); // NOTE: This is a bit sketchy: this method adjusts the inlines as a // side effect, which means it will ultimately adjust the transaction // comments and affect timeline rendering. $query->adjustInlinesForChangesets( $inlines, array_select_keys($changesets, $old_ids), array_select_keys($changesets, $new_ids), $this); return $timeline ->setChangesets($changesets) ->setRevision($this) ->setLeftDiff($left_diff) ->setRightDiff($right_diff); } /* -( PhabricatorDestructibleInterface )----------------------------------- */ public function destroyObjectPermanently( PhabricatorDestructionEngine $engine) { $this->openTransaction(); $diffs = id(new DifferentialDiffQuery()) ->setViewer($engine->getViewer()) ->withRevisionIDs(array($this->getID())) ->execute(); foreach ($diffs as $diff) { $engine->destroyObject($diff); } $conn_w = $this->establishConnection('w'); queryfx( $conn_w, 'DELETE FROM %T WHERE revisionID = %d', self::TABLE_COMMIT, $this->getID()); // we have to do paths a little differentally as they do not have // an id or phid column for delete() to act on $dummy_path = new DifferentialAffectedPath(); queryfx( $conn_w, 'DELETE FROM %T WHERE revisionID = %d', $dummy_path->getTableName(), $this->getID()); $this->delete(); $this->saveTransaction(); } /* -( PhabricatorFulltextInterface )--------------------------------------- */ public function newFulltextEngine() { return new DifferentialRevisionFulltextEngine(); } /* -( PhabricatorConduitResultInterface )---------------------------------- */ public function getFieldSpecificationsForConduit() { return array( id(new PhabricatorConduitSearchFieldSpecification()) ->setKey('title') ->setType('string') ->setDescription(pht('The revision title.')), id(new PhabricatorConduitSearchFieldSpecification()) ->setKey('authorPHID') ->setType('phid') ->setDescription(pht('Revision author PHID.')), ); } public function getFieldValuesForConduit() { return array( 'title' => $this->getTitle(), 'authorPHID' => $this->getAuthorPHID(), ); } public function getConduitSearchAttachments() { return array( id(new DifferentialReviewersSearchEngineAttachment()) ->setAttachmentKey('reviewers'), ); } /* -( PhabricatorDraftInterface )------------------------------------------ */ public function newDraftEngine() { return new DifferentialRevisionDraftEngine(); } } diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitFieldsQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitFieldsQuery.php index ab8f5e3e2e..a0548d5178 100644 --- a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitFieldsQuery.php +++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitFieldsQuery.php @@ -1,143 +1,142 @@ null, 'foundURI' => null, 'validDomain' => null, 'matchHashType' => null, 'matchHashValue' => null, ); public function withCommitRef(DiffusionCommitRef $ref) { $this->ref = $ref; return $this; } public function getRevisionMatchData() { return $this->revisionMatchData; } private function setRevisionMatchData($key, $value) { $this->revisionMatchData[$key] = $value; return $this; } protected function executeQuery() { $ref = $this->ref; $message = $ref->getMessage(); $hashes = $ref->getHashes(); $params = array( 'corpus' => $message, 'partial' => true, ); $result = id(new ConduitCall('differential.parsecommitmessage', $params)) ->setUser(PhabricatorUser::getOmnipotentUser()) ->execute(); $fields = $result['fields']; $revision_id = idx($fields, 'revisionID'); if ($revision_id) { $this->setRevisionMatchData('usedURI', true); } else { $this->setRevisionMatchData('usedURI', false); } $revision_id_info = $result['revisionIDFieldInfo']; $this->setRevisionMatchData('foundURI', $revision_id_info['value']); $this->setRevisionMatchData( 'validDomain', $revision_id_info['validDomain']); // If there is no "Differential Revision:" field in the message, try to // identify the revision by doing a hash lookup. if (!$revision_id && $hashes) { $hash_list = array(); foreach ($hashes as $hash) { $hash_list[] = array($hash->getHashType(), $hash->getHashValue()); } $revisions = id(new DifferentialRevisionQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) ->needHashes(true) ->withCommitHashes($hash_list) ->execute(); if ($revisions) { $revision = $this->pickBestRevision($revisions); $fields['revisionID'] = $revision->getID(); $revision_hashes = $revision->getHashes(); $revision_hashes = DiffusionCommitHash::convertArrayToObjects( $revision_hashes); $revision_hashes = mpull($revision_hashes, null, 'getHashType'); // sort the hashes in the order the mighty // @{class:ArcanstDifferentialRevisionHash} does; probably unnecessary // but should future proof things nicely. $revision_hashes = array_select_keys( $revision_hashes, ArcanistDifferentialRevisionHash::getTypes()); foreach ($hashes as $hash) { $revision_hash = idx($revision_hashes, $hash->getHashType()); if (!$revision_hash) { continue; } if ($revision_hash->getHashValue() == $hash->getHashValue()) { $this->setRevisionMatchData( 'matchHashType', $hash->getHashType()); $this->setRevisionMatchData( 'matchHashValue', $hash->getHashValue()); break; } } } } return $fields; } /** * When querying for revisions by hash, more than one revision may be found. * This function identifies the "best" revision from such a set. Typically, * there is only one revision found. Otherwise, we try to pick an accepted * revision first, followed by an open revision, and otherwise we go with a * closed or abandoned revision as a last resort. */ private function pickBestRevision(array $revisions) { assert_instances_of($revisions, 'DifferentialRevision'); // If we have more than one revision of a given status, choose the most // recently updated one. $revisions = msort($revisions, 'getDateModified'); $revisions = array_reverse($revisions); // Try to find an accepted revision first. - $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; foreach ($revisions as $revision) { - if ($revision->getStatus() == $status_accepted) { + if ($revision->isAccepted()) { return $revision; } } // Try to find an open revision. foreach ($revisions as $revision) { if (!$revision->isClosed()) { return $revision; } } // Settle for whatever's left. return head($revisions); } } diff --git a/src/applications/drydock/operation/DrydockLandRepositoryOperation.php b/src/applications/drydock/operation/DrydockLandRepositoryOperation.php index 4e306772a3..1ccc82eb7b 100644 --- a/src/applications/drydock/operation/DrydockLandRepositoryOperation.php +++ b/src/applications/drydock/operation/DrydockLandRepositoryOperation.php @@ -1,443 +1,442 @@ getRepositoryTarget(); $repository = $operation->getRepository(); switch ($operation->getOperationState()) { case DrydockRepositoryOperation::STATE_WAIT: return pht( 'Waiting to land revision into %s on %s...', $repository->getMonogram(), $target); case DrydockRepositoryOperation::STATE_WORK: return pht( 'Landing revision into %s on %s...', $repository->getMonogram(), $target); case DrydockRepositoryOperation::STATE_DONE: return pht( 'Revision landed into %s.', $repository->getMonogram()); } } public function getWorkingCopyMerges(DrydockRepositoryOperation $operation) { $repository = $operation->getRepository(); $merges = array(); $object = $operation->getObject(); if ($object instanceof DifferentialRevision) { $diff = $this->loadDiff($operation); $merges[] = array( 'src.uri' => $repository->getStagingURI(), 'src.ref' => $diff->getStagingRef(), ); } else { throw new Exception( pht( 'Invalid or unknown object ("%s") for land operation, expected '. 'Differential Revision.', $operation->getObjectPHID())); } return $merges; } public function applyOperation( DrydockRepositoryOperation $operation, DrydockInterface $interface) { $viewer = $this->getViewer(); $repository = $operation->getRepository(); $cmd = array(); $arg = array(); $object = $operation->getObject(); if ($object instanceof DifferentialRevision) { $revision = $object; $diff = $this->loadDiff($operation); $dict = $diff->getDiffAuthorshipDict(); $author_name = idx($dict, 'authorName'); $author_email = idx($dict, 'authorEmail'); $api_method = 'differential.getcommitmessage'; $api_params = array( 'revision_id' => $revision->getID(), ); $commit_message = id(new ConduitCall($api_method, $api_params)) ->setUser($viewer) ->execute(); } else { throw new Exception( pht( 'Invalid or unknown object ("%s") for land operation, expected '. 'Differential Revision.', $operation->getObjectPHID())); } $target = $operation->getRepositoryTarget(); list($type, $name) = explode(':', $target, 2); switch ($type) { case 'branch': $push_dst = 'refs/heads/'.$name; break; default: throw new Exception( pht( 'Unknown repository operation target type "%s" (in target "%s").', $type, $target)); } $committer_info = $this->getCommitterInfo($operation); // NOTE: We're doing this commit with "-F -" so we don't run into trouble // with enormous commit messages which might otherwise exceed the maximum // size of a command. $future = $interface->getExecFuture( 'git -c user.name=%s -c user.email=%s commit --author %s -F - --', $committer_info['name'], $committer_info['email'], "{$author_name} <{$author_email}>"); $future->write($commit_message); try { $future->resolvex(); } catch (CommandException $ex) { $display_command = csprintf('git commit'); // TODO: One reason this can fail is if the changes have already been // merged. We could try to detect that. $error = DrydockCommandError::newFromCommandException($ex) ->setPhase(self::PHASE_COMMIT) ->setDisplayCommand($display_command); $operation->setCommandError($error->toDictionary()); throw $ex; } try { $interface->execx( 'git push origin -- %s:%s', 'HEAD', $push_dst); } catch (CommandException $ex) { $display_command = csprintf( 'git push origin %R:%R', 'HEAD', $push_dst); $error = DrydockCommandError::newFromCommandException($ex) ->setPhase(self::PHASE_PUSH) ->setDisplayCommand($display_command); $operation->setCommandError($error->toDictionary()); throw $ex; } } private function getCommitterInfo(DrydockRepositoryOperation $operation) { $viewer = $this->getViewer(); $committer_name = null; $author_phid = $operation->getAuthorPHID(); $object = id(new PhabricatorObjectQuery()) ->setViewer($viewer) ->withPHIDs(array($author_phid)) ->executeOne(); if ($object) { if ($object instanceof PhabricatorUser) { $committer_name = $object->getUsername(); } } if (!strlen($committer_name)) { $committer_name = pht('autocommitter'); } // TODO: Probably let users choose a VCS email address in settings. For // now just make something up so we don't leak anyone's stuff. return array( 'name' => $committer_name, 'email' => 'autocommitter@example.com', ); } private function loadDiff(DrydockRepositoryOperation $operation) { $viewer = $this->getViewer(); $revision = $operation->getObject(); $diff_phid = $operation->getProperty('differential.diffPHID'); $diff = id(new DifferentialDiffQuery()) ->setViewer($viewer) ->withPHIDs(array($diff_phid)) ->executeOne(); if (!$diff) { throw new Exception( pht( 'Unable to load diff "%s".', $diff_phid)); } $diff_revid = $diff->getRevisionID(); $revision_id = $revision->getID(); if ($diff_revid != $revision_id) { throw new Exception( pht( 'Diff ("%s") has wrong revision ID ("%s", expected "%s").', $diff_phid, $diff_revid, $revision_id)); } return $diff; } public function getBarrierToLanding( PhabricatorUser $viewer, DifferentialRevision $revision) { $repository = $revision->getRepository(); if (!$repository) { return array( 'title' => pht('No Repository'), 'body' => pht( 'This revision is not associated with a known repository. Only '. 'revisions associated with a tracked repository can be landed '. 'automatically.'), ); } if (!$repository->canPerformAutomation()) { return array( 'title' => pht('No Repository Automation'), 'body' => pht( 'The repository this revision is associated with ("%s") is not '. 'configured to support automation. Configure automation for the '. 'repository to enable revisions to be landed automatically.', $repository->getMonogram()), ); } // Check if this diff was pushed to a staging area. $diff = id(new DifferentialDiffQuery()) ->setViewer($viewer) ->withIDs(array($revision->getActiveDiff()->getID())) ->needProperties(true) ->executeOne(); // Older diffs won't have this property. They may still have been pushed. // At least for now, assume staging changes are present if the property // is missing. This should smooth the transition to the more formal // approach. $has_staging = $diff->hasDiffProperty('arc.staging'); if ($has_staging) { $staging = $diff->getProperty('arc.staging'); if (!is_array($staging)) { $staging = array(); } $status = idx($staging, 'status'); if ($status != ArcanistDiffWorkflow::STAGING_PUSHED) { return $this->getBarrierToLandingFromStagingStatus($status); } } // TODO: At some point we should allow installs to give "land reviewed // code" permission to more users than "push any commit", because it is // a much less powerful operation. For now, just require push so this // doesn't do anything users can't do on their own. $can_push = PhabricatorPolicyFilter::hasCapability( $viewer, $repository, DiffusionPushCapability::CAPABILITY); if (!$can_push) { return array( 'title' => pht('Unable to Push'), 'body' => pht( 'You do not have permission to push to the repository this '. 'revision is associated with ("%s"), so you can not land it.', $repository->getMonogram()), ); } - $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; - if ($revision->getStatus() != $status_accepted) { - switch ($revision->getStatus()) { - case ArcanistDifferentialRevisionStatus::CLOSED: - return array( - 'title' => pht('Revision Closed'), - 'body' => pht( - 'This revision has already been closed. Only open, accepted '. - 'revisions may land.'), - ); - case ArcanistDifferentialRevisionStatus::ABANDONED: - return array( - 'title' => pht('Revision Abandoned'), - 'body' => pht( - 'This revision has been abandoned. Only accepted revisions '. - 'may land.'), - ); - default: - return array( - 'title' => pht('Revision Not Accepted'), - 'body' => pht( - 'This revision is still under review. Only revisions which '. - 'have been accepted may land.'), - ); - } + if ($revision->isAccepted()) { + // We can land accepted revisions, so continue below. Otherwise, raise + // an error with tailored messaging for the most common cases. + } else if ($revision->isAbandoned()) { + return array( + 'title' => pht('Revision Abandoned'), + 'body' => pht( + 'This revision has been abandoned. Only accepted revisions '. + 'may land.'), + ); + } else if ($revision->isClosed()) { + return array( + 'title' => pht('Revision Closed'), + 'body' => pht( + 'This revision has already been closed. Only open, accepted '. + 'revisions may land.'), + ); + } else { + return array( + 'title' => pht('Revision Not Accepted'), + 'body' => pht( + 'This revision is still under review. Only revisions which '. + 'have been accepted may land.'), + ); } // Check for other operations. Eventually this should probably be more // general (e.g., it's OK to land to multiple different branches // simultaneously) but just put this in as a sanity check for now. $other_operations = id(new DrydockRepositoryOperationQuery()) ->setViewer($viewer) ->withObjectPHIDs(array($revision->getPHID())) ->withOperationTypes( array( $this->getOperationConstant(), )) ->withOperationStates( array( DrydockRepositoryOperation::STATE_WAIT, DrydockRepositoryOperation::STATE_WORK, DrydockRepositoryOperation::STATE_DONE, )) ->execute(); if ($other_operations) { $any_done = false; foreach ($other_operations as $operation) { if ($operation->isDone()) { $any_done = true; break; } } if ($any_done) { return array( 'title' => pht('Already Complete'), 'body' => pht('This revision has already landed.'), ); } else { return array( 'title' => pht('Already In Flight'), 'body' => pht('This revision is already landing.'), ); } } return null; } private function getBarrierToLandingFromStagingStatus($status) { switch ($status) { case ArcanistDiffWorkflow::STAGING_USER_SKIP: return array( 'title' => pht('Staging Area Skipped'), 'body' => pht( 'The diff author used the %s flag to skip pushing this change to '. 'staging. Changes must be pushed to staging before they can be '. 'landed from the web.', phutil_tag('tt', array(), '--skip-staging')), ); case ArcanistDiffWorkflow::STAGING_DIFF_RAW: return array( 'title' => pht('Raw Diff Source'), 'body' => pht( 'The diff was generated from a raw input source, so the change '. 'could not be pushed to staging. Changes must be pushed to '. 'staging before they can be landed from the web.'), ); case ArcanistDiffWorkflow::STAGING_REPOSITORY_UNKNOWN: return array( 'title' => pht('Unknown Repository'), 'body' => pht( 'When the diff was generated, the client was not able to '. 'determine which repository it belonged to, so the change '. 'was not pushed to staging. Changes must be pushed to staging '. 'before they can be landed from the web.'), ); case ArcanistDiffWorkflow::STAGING_REPOSITORY_UNAVAILABLE: return array( 'title' => pht('Staging Unavailable'), 'body' => pht( 'When this diff was generated, the server was running an older '. 'version of Phabricator which did not support staging areas, so '. 'the change was not pushed to staging. Changes must be pushed '. 'to staging before they can be landed from the web.'), ); case ArcanistDiffWorkflow::STAGING_REPOSITORY_UNSUPPORTED: return array( 'title' => pht('Repository Unsupported'), 'body' => pht( 'When this diff was generated, the server was running an older '. 'version of Phabricator which did not support staging areas for '. 'this version control system, so the chagne was not pushed to '. 'staging. Changes must be pushed to staging before they can be '. 'landed from the web.'), ); case ArcanistDiffWorkflow::STAGING_REPOSITORY_UNCONFIGURED: return array( 'title' => pht('Repository Unconfigured'), 'body' => pht( 'When this diff was generated, the repository was not configured '. 'with a staging area, so the change was not pushed to staging. '. 'Changes must be pushed to staging before they can be landed '. 'from the web.'), ); case ArcanistDiffWorkflow::STAGING_CLIENT_UNSUPPORTED: return array( 'title' => pht('Client Support Unavailable'), 'body' => pht( 'When this diff was generated, the client did not support '. 'staging areas for this version control system, so the change '. 'was not pushed to staging. Changes must be pushed to staging '. 'before they can be landed from the web. Updating the client '. 'may resolve this issue.'), ); default: return array( 'title' => pht('Unknown Error'), 'body' => pht( 'When this diff was generated, it was not pushed to staging for '. 'an unknown reason (the status code was "%s"). Changes must be '. 'pushed to staging before they can be landed from the web. '. 'The server may be running an out-of-date version of Phabricator, '. 'and updating may provide more information about this error.', $status), ); } } } diff --git a/src/applications/releeph/differential/DifferentialReleephRequestFieldSpecification.php b/src/applications/releeph/differential/DifferentialReleephRequestFieldSpecification.php index bdb63e6c9c..dd67f5bc59 100644 --- a/src/applications/releeph/differential/DifferentialReleephRequestFieldSpecification.php +++ b/src/applications/releeph/differential/DifferentialReleephRequestFieldSpecification.php @@ -1,385 +1,384 @@ " headers in commits created by * arc-releeph so that RQs committed by arc-releeph have real * PhabricatorRepositoryCommits associated with them (instaed of just the SHA * of the commit, as seen by the pusher). * * 2: If requestors want to commit directly to their release branch, they can * use this header to (i) indicate on a differential revision that this * differential revision is for the release branch, and (ii) when they land * their diff on to the release branch manually, the ReleephRequest is * automatically updated (instead of having to use the "Mark Manually Picked" * button.) * */ final class DifferentialReleephRequestFieldSpecification extends Phobject { // TODO: This class is essentially dead right now, see T2222. const ACTION_PICKS = 'picks'; const ACTION_REVERTS = 'reverts'; private $releephAction; private $releephPHIDs = array(); public function getStorageKey() { return 'releeph:actions'; } public function getValueForStorage() { return json_encode(array( 'releephAction' => $this->releephAction, 'releephPHIDs' => $this->releephPHIDs, )); } public function setValueFromStorage($json) { if ($json) { $dict = phutil_json_decode($json); $this->releephAction = idx($dict, 'releephAction'); $this->releephPHIDs = idx($dict, 'releephPHIDs'); } return $this; } public function shouldAppearOnRevisionView() { return true; } public function renderLabelForRevisionView() { return pht('Releeph'); } public function getRequiredHandlePHIDs() { return mpull($this->loadReleephRequests(), 'getPHID'); } public function renderValueForRevisionView() { static $tense; if ($tense === null) { $tense = array( self::ACTION_PICKS => array( 'future' => pht('Will pick'), 'past' => pht('Picked'), ), self::ACTION_REVERTS => array( 'future' => pht('Will revert'), 'past' => pht('Reverted'), ), ); } $releeph_requests = $this->loadReleephRequests(); if (!$releeph_requests) { return null; } - $status = $this->getRevision()->getStatus(); - if ($status == ArcanistDifferentialRevisionStatus::CLOSED) { + if ($this->getRevision()->isClosed()) { $verb = $tense[$this->releephAction]['past']; } else { $verb = $tense[$this->releephAction]['future']; } $parts = hsprintf('%s...', $verb); foreach ($releeph_requests as $releeph_request) { $parts->appendHTML(phutil_tag('br')); $parts->appendHTML( $this->getHandle($releeph_request->getPHID())->renderLink()); } return $parts; } public function shouldAppearOnCommitMessage() { return true; } public function getCommitMessageKey() { return 'releephActions'; } public function setValueFromParsedCommitMessage($dict) { $this->releephAction = $dict['releephAction']; $this->releephPHIDs = $dict['releephPHIDs']; return $this; } public function renderValueForCommitMessage($is_edit) { $releeph_requests = $this->loadReleephRequests(); if (!$releeph_requests) { return null; } $parts = array($this->releephAction); foreach ($releeph_requests as $releeph_request) { $parts[] = 'RQ'.$releeph_request->getID(); } return implode(' ', $parts); } /** * Releeph fields should look like: * * Releeph: picks RQ1 RQ2, RQ3 * Releeph: reverts RQ1 */ public function parseValueFromCommitMessage($value) { /** * Releeph commit messages look like this (but with more blank lines, * omitted here): * * Make CaptainHaddock more reasonable * Releeph: picks RQ1 * Requested By: edward * Approved By: edward (requestor) * Request Reason: x * Summary: Make the Haddock implementation more reasonable. * Test Plan: none * Reviewers: user1 * * Some of these fields are recognized by Differential (e.g. "Requested * By"). They are folded up into the "Releeph" field, parsed by this * class. As such $value includes more than just the first-line: * * "picks RQ1\n\nRequested By: edward\n\nApproved By: edward (requestor)" * * To hack around this, just consider the first line of $value when * determining what Releeph actions the parsed commit is performing. */ $first_line = head(array_filter(explode("\n", $value))); $tokens = preg_split('/\s*,?\s+/', $first_line); $raw_action = array_shift($tokens); $action = strtolower($raw_action); if (!$action) { return null; } switch ($action) { case self::ACTION_REVERTS: case self::ACTION_PICKS: break; default: throw new DifferentialFieldParseException( pht( "Commit message contains unknown Releeph action '%s'!", $raw_action)); break; } $releeph_requests = array(); foreach ($tokens as $token) { $match = array(); if (!preg_match('/^(?:RQ)?(\d+)$/i', $token, $match)) { $label = $this->renderLabelForCommitMessage(); throw new DifferentialFieldParseException( pht( "Commit message contains unparseable ". "Releeph request token '%s'!", $token)); } $id = (int)$match[1]; $releeph_request = id(new ReleephRequest())->load($id); if (!$releeph_request) { throw new DifferentialFieldParseException( pht( 'Commit message references non existent Releeph request: %s!', $value)); } $releeph_requests[] = $releeph_request; } if (count($releeph_requests) > 1) { $rqs_seen = array(); $groups = array(); foreach ($releeph_requests as $releeph_request) { $releeph_branch = $releeph_request->getBranch(); $branch_name = $releeph_branch->getName(); $rq_id = 'RQ'.$releeph_request->getID(); if (idx($rqs_seen, $rq_id)) { throw new DifferentialFieldParseException( pht( 'Commit message refers to %s multiple times!', $rq_id)); } $rqs_seen[$rq_id] = true; if (!isset($groups[$branch_name])) { $groups[$branch_name] = array(); } $groups[$branch_name][] = $rq_id; } if (count($groups) > 1) { $lists = array(); foreach ($groups as $branch_name => $rq_ids) { $lists[] = implode(', ', $rq_ids).' in '.$branch_name; } throw new DifferentialFieldParseException( pht( 'Commit message references multiple Releeph requests, '. 'but the requests are in different branches: %s', implode('; ', $lists))); } } $phids = mpull($releeph_requests, 'getPHID'); $data = array( 'releephAction' => $action, 'releephPHIDs' => $phids, ); return $data; } public function renderLabelForCommitMessage() { return pht('Releeph'); } public function shouldAppearOnCommitMessageTemplate() { return false; } public function didParseCommit( PhabricatorRepository $repo, PhabricatorRepositoryCommit $commit, PhabricatorRepositoryCommitData $data) { // NOTE: This is currently dead code. See T2222. $releeph_requests = $this->loadReleephRequests(); if (!$releeph_requests) { return; } $releeph_branch = head($releeph_requests)->getBranch(); if (!$this->isCommitOnBranch($repo, $commit, $releeph_branch)) { return; } foreach ($releeph_requests as $releeph_request) { if ($this->releephAction === self::ACTION_PICKS) { $action = 'pick'; } else { $action = 'revert'; } $actor_phid = coalesce( $data->getCommitDetail('committerPHID'), $data->getCommitDetail('authorPHID')); $actor = id(new PhabricatorUser()) ->loadOneWhere('phid = %s', $actor_phid); $xactions = array(); $xactions[] = id(new ReleephRequestTransaction()) ->setTransactionType(ReleephRequestTransaction::TYPE_DISCOVERY) ->setMetadataValue('action', $action) ->setMetadataValue('authorPHID', $data->getCommitDetail('authorPHID')) ->setMetadataValue('committerPHID', $data->getCommitDetail('committerPHID')) ->setNewValue($commit->getPHID()); $editor = id(new ReleephRequestTransactionalEditor()) ->setActor($actor) ->setContinueOnNoEffect(true) ->setContentSource( PhabricatorContentSource::newForSource( PhabricatorUnknownContentSource::SOURCECONST)); $editor->applyTransactions($releeph_request, $xactions); } } private function loadReleephRequests() { if (!$this->releephPHIDs) { return array(); } return id(new ReleephRequestQuery()) ->setViewer($this->getViewer()) ->withPHIDs($this->releephPHIDs) ->execute(); } private function isCommitOnBranch( PhabricatorRepository $repo, PhabricatorRepositoryCommit $commit, ReleephBranch $releeph_branch) { switch ($repo->getVersionControlSystem()) { case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: list($output) = $repo->execxLocalCommand( 'branch --all --no-color --contains %s', $commit->getCommitIdentifier()); $remote_prefix = 'remotes/origin/'; $branches = array(); foreach (array_filter(explode("\n", $output)) as $line) { $tokens = explode(' ', $line); $ref = last($tokens); if (strncmp($ref, $remote_prefix, strlen($remote_prefix)) === 0) { $branch = substr($ref, strlen($remote_prefix)); $branches[$branch] = $branch; } } return idx($branches, $releeph_branch->getName()); break; case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: $change_query = DiffusionPathChangeQuery::newFromDiffusionRequest( DiffusionRequest::newFromDictionary(array( 'user' => $this->getUser(), 'repository' => $repo, 'commit' => $commit->getCommitIdentifier(), ))); $path_changes = $change_query->loadChanges(); $commit_paths = mpull($path_changes, 'getPath'); $branch_path = $releeph_branch->getName(); $in_branch = array(); $ex_branch = array(); foreach ($commit_paths as $path) { if (strncmp($path, $branch_path, strlen($branch_path)) === 0) { $in_branch[] = $path; } else { $ex_branch[] = $path; } } if ($in_branch && $ex_branch) { $error = pht( 'CONFUSION: commit %s in %s contains %d path change(s) that were '. 'part of a Releeph branch, but also has %d path change(s) not '. 'part of a Releeph branch!', $commit->getCommitIdentifier(), $repo->getDisplayName(), count($in_branch), count($ex_branch)); phlog($error); } return !empty($in_branch); break; } } }