diff --git a/resources/sql/autopatches/20140420.rel.1.objectphid.sql b/resources/sql/autopatches/20140420.rel.1.objectphid.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20140420.rel.1.objectphid.sql @@ -0,0 +1,5 @@ +ALTER TABLE {$NAMESPACE}_releeph.releeph_request + ADD COLUMN requestedObjectPHID VARCHAR(64) COLLATE utf8_bin NOT NULL; + +ALTER TABLE {$NAMESPACE}_releeph.releeph_request + ADD KEY `key_requestedObject` (requestedObjectPHID); diff --git a/resources/sql/autopatches/20140420.rel.2.objectmig.php b/resources/sql/autopatches/20140420.rel.2.objectmig.php new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20140420.rel.2.objectmig.php @@ -0,0 +1,45 @@ +getTableName(); +$conn_w = $pull_table->establishConnection('w'); + +echo "Setting object PHIDs for requests...\n"; +foreach (new LiskMigrationIterator($pull_table) as $pull) { + $id = $pull->getID(); + + echo "Migrating pull request {$id}...\n"; + if ($pull->getRequestedObjectPHID()) { + // We already have a valid PHID, so skip this request. + continue; + } + + $commit_phids = $pull->getCommitPHIDs(); + if (count($commit_phids) != 1) { + // At the time this migration was written, all requests had exactly one + // commit. If a request has more than one, we don't have the information + // we need to process it. + continue; + } + + $commit_phid = head($commit_phids); + + $revision_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( + $commit_phid, + PhabricatorEdgeConfig::TYPE_COMMIT_HAS_DREV); + + if ($revision_phids) { + $object_phid = head($revision_phids); + } else { + $object_phid = $commit_phid; + } + + queryfx( + $conn_w, + 'UPDATE %T SET requestedObjectPHID = %s WHERE id = %d', + $table_name, + $object_phid, + $id); +} + +echo "Done.\n"; diff --git a/src/applications/releeph/commitfinder/ReleephCommitFinder.php b/src/applications/releeph/commitfinder/ReleephCommitFinder.php --- a/src/applications/releeph/commitfinder/ReleephCommitFinder.php +++ b/src/applications/releeph/commitfinder/ReleephCommitFinder.php @@ -4,6 +4,7 @@ private $releephProject; private $user; + private $objectPHID; public function setUser(PhabricatorUser $user) { $this->user = $user; @@ -18,7 +19,13 @@ return $this; } + public function getRequestedObjectPHID() { + return $this->objectPHID; + } + public function fromPartial($partial_string) { + $this->objectPHID = null; + // Look for diffs $matches = array(); if (preg_match('/^D([1-9]\d*)$/', $partial_string, $matches)) { @@ -36,6 +43,8 @@ "{$partial_string} has no commits associated with it yet."); } + $this->objectPHID = $diff_rev->getPHID(); + $commits = id(new PhabricatorRepositoryCommit())->loadAllWhere( 'phid IN (%Ls) ORDER BY epoch ASC', $commit_phids); @@ -43,7 +52,7 @@ } // Look for a raw commit number, or r. - $repository = $this->releephProject->loadPhabricatorRepository(); + $repository = $this->releephProject->getRepository(); $dr_data = null; $matches = array(); if (preg_match('/^r(?P[A-Z]+)(?P\w+)$/', @@ -79,6 +88,17 @@ "The commit {$partial_string} doesn't exist in this repository."); } + // When requesting a single commit, if it has an associated review we + // imply the review was requested instead. This is always correct for now + // and consistent with the older behavior, although it might not be the + // right rule in the future. + $phids = PhabricatorEdgeQuery::loadDestinationPHIDs( + $phabricator_repository_commit->getPHID(), + PhabricatorEdgeConfig::TYPE_COMMIT_HAS_DREV); + if ($phids) { + $this->objectPHID = head($phids); + } + return $phabricator_repository_commit; } diff --git a/src/applications/releeph/conduit/ConduitAPI_releeph_request_Method.php b/src/applications/releeph/conduit/ConduitAPI_releeph_request_Method.php --- a/src/applications/releeph/conduit/ConduitAPI_releeph_request_Method.php +++ b/src/applications/releeph/conduit/ConduitAPI_releeph_request_Method.php @@ -49,6 +49,7 @@ // Find the requested commit identifiers $requested_commits = array(); + $requested_object_phids = array(); $things = $request->getValue('things'); $finder = id(new ReleephCommitFinder()) ->setUser($user) @@ -56,6 +57,11 @@ foreach ($things as $thing) { try { $requested_commits[$thing] = $finder->fromPartial($thing); + $object_phid = $finder->getRequestedObjectPHID(); + if (!$object_phid) { + $object_phid = $requested_commits[$thing]->getPHID(); + } + $requested_object_phids[$thing] = $object_phid; } catch (ReleephCommitFinderException $ex) { throw id(new ConduitException('ERR_NO_MATCHES')) ->setErrorDescription($ex->getMessage()); @@ -99,7 +105,8 @@ $releeph_request = id(new ReleephRequest()) ->setRequestUserPHID($user->getPHID()) ->setBranchID($releeph_branch->getID()) - ->setInBranch(0); + ->setInBranch(0) + ->setRequestedObjectPHID($requested_object_phids[$thing]); $xactions = array(); diff --git a/src/applications/releeph/controller/request/ReleephRequestEditController.php b/src/applications/releeph/controller/request/ReleephRequestEditController.php --- a/src/applications/releeph/controller/request/ReleephRequestEditController.php +++ b/src/applications/releeph/controller/request/ReleephRequestEditController.php @@ -43,7 +43,8 @@ $pull = id(new ReleephRequest()) ->setRequestUserPHID($viewer->getPHID()) ->setBranchID($branch->getID()) - ->setInBranch(0); + ->setInBranch(0) + ->attachBranch($branch); $is_edit = false; } @@ -116,6 +117,15 @@ $errors[] = "The requested commit hasn't been parsed yet."; } } + + if (!$errors) { + $object_phid = $finder->getRequestedObjectPHID(); + if (!$object_phid) { + $object_phid = $pr_commit->getPHID(); + } + + $pull->setRequestedObjectPHID($object_phid); + } } if (!$errors) { diff --git a/src/applications/releeph/storage/ReleephRequest.php b/src/applications/releeph/storage/ReleephRequest.php --- a/src/applications/releeph/storage/ReleephRequest.php +++ b/src/applications/releeph/storage/ReleephRequest.php @@ -13,6 +13,13 @@ protected $pickStatus; protected $mailKey; + /** + * The object which is being requested. Normally this is a commit, but it + * might also be a revision. In the future, it could be a repository branch + * or an external object (like a GitHub pull request). + */ + protected $requestedObjectPHID; + // Information about the thing being requested protected $requestCommitPHID; @@ -20,6 +27,7 @@ protected $commitIdentifier; protected $commitPHID; + private $customFields = self::ATTACHABLE; private $branch = self::ATTACHABLE; @@ -163,6 +171,20 @@ return $this; } + + /** + * Get the commit PHIDs this request is requesting. + * + * NOTE: For now, this always returns one PHID. + * + * @return list Commit PHIDs requested by this request. + */ + public function getCommitPHIDs() { + return array( + $this->requestCommitPHID, + ); + } + public function getReason() { // Backward compatibility: reason used to be called comments $reason = $this->getDetail('reason');