From fcb728653312789fe5afdfc16c2f72724c8484de Mon Sep 17 00:00:00 2001 From: Edward Speyer Date: Mon, 8 Apr 2013 16:34:21 +0100 Subject: [PATCH] ReleephRequest xactions Summary: Migrate to `PhabricatorApplicationTransactions` (`ReleephRequestTransactions` applied by `ReleephRequestTransactionalEditor`, instead of `ReleephRequestEvents` created by `ReleephRequestEditor`) and migrate all the old events into transactions. Email is supported in the standard way (no more `ReleephRequestMail`) as well. This also collapses the Releeph request create and edit controllers into one class, as well as breaking everyone's subject-based mail rules by standardising them (but which should be more easily filtered by looking at headers.) Test Plan: * Make requests, then pick them. * Pick and revert the same request so that discovery happens way after `arc` has told Releeph about what's been happening. * Try to pick something that fails to pick in a project with pick instructions (and see the instructions are in the email.) * Load all of FB's Releeph data into my DB and run the `storage upgrade` script. * Request a commit via the "action" in a Differential revision. Reviewers: epriestley Reviewed By: epriestley CC: epriestley, aran, Korvin, wez Maniphest Tasks: T3092, T2720 Differential Revision: https://secure.phabricator.com/D5868 --- .../patches/20130508.releephtransactions.sql | 38 ++ .../20130508.releephtransactionsmig.php | 131 ++++++ src/__phutil_library_map__.php | 19 +- .../PhabricatorApplicationReleeph.php | 6 +- .../ConduitAPI_releeph_request_Method.php | 38 +- .../ConduitAPI_releephwork_record_Method.php | 33 +- ...PI_releephwork_recordpickstatus_Method.php | 20 +- .../ReleephRequestActionController.php | 52 ++- .../ReleephRequestCommentController.php | 63 +++ .../ReleephRequestCreateController.php | 165 ------- ...ephRequestDifferentialCreateController.php | 4 +- .../request/ReleephRequestEditController.php | 264 +++++++++--- .../request/ReleephRequestViewController.php | 65 +-- ...entialReleephRequestFieldSpecification.php | 21 +- .../releeph/editor/ReleephRequestEditor.php | 402 ------------------ .../ReleephRequestTransactionalEditor.php | 300 +++++++++++++ .../editor/mail/ReleephRequestMail.php | 213 ---------- .../ReleephFieldSpecification.php | 50 ++- .../mail/ReleephRequestReplyHandler.php | 55 +++ .../query/ReleephRequestTransactionQuery.php | 10 + .../releeph/storage/ReleephRequest.php | 7 - .../storage/ReleephRequestTransaction.php | 283 ++++++++++++ .../ReleephRequestTransactionComment.php | 10 + .../ReleephRequestTypeaheadControl.php | 4 +- .../ReleephRequestEventListView.php | 266 ------------ .../patch/PhabricatorBuiltinPatchList.php | 8 + 26 files changed, 1326 insertions(+), 1201 deletions(-) create mode 100644 resources/sql/patches/20130508.releephtransactions.sql create mode 100644 resources/sql/patches/20130508.releephtransactionsmig.php create mode 100644 src/applications/releeph/controller/request/ReleephRequestCommentController.php delete mode 100644 src/applications/releeph/controller/request/ReleephRequestCreateController.php delete mode 100644 src/applications/releeph/editor/ReleephRequestEditor.php create mode 100644 src/applications/releeph/editor/ReleephRequestTransactionalEditor.php delete mode 100644 src/applications/releeph/editor/mail/ReleephRequestMail.php create mode 100644 src/applications/releeph/mail/ReleephRequestReplyHandler.php create mode 100644 src/applications/releeph/query/ReleephRequestTransactionQuery.php create mode 100644 src/applications/releeph/storage/ReleephRequestTransaction.php create mode 100644 src/applications/releeph/storage/ReleephRequestTransactionComment.php delete mode 100644 src/applications/releeph/view/requestevent/ReleephRequestEventListView.php diff --git a/resources/sql/patches/20130508.releephtransactions.sql b/resources/sql/patches/20130508.releephtransactions.sql new file mode 100644 index 0000000000..0da0bc8a7d --- /dev/null +++ b/resources/sql/patches/20130508.releephtransactions.sql @@ -0,0 +1,38 @@ +CREATE TABLE {$NAMESPACE}_releeph.releeph_requesttransaction ( + id INT UNSIGNED NOT NULL PRIMARY KEY AUTO_INCREMENT, + phid VARCHAR(64) NOT NULL COLLATE utf8_bin, + authorPHID VARCHAR(64) NOT NULL COLLATE utf8_bin, + objectPHID VARCHAR(64) NOT NULL COLLATE utf8_bin, + viewPolicy VARCHAR(64) NOT NULL COLLATE utf8_bin, + editPolicy VARCHAR(64) NOT NULL COLLATE utf8_bin, + commentPHID VARCHAR(64) COLLATE utf8_bin, + commentVersion INT UNSIGNED NOT NULL, + transactionType VARCHAR(32) NOT NULL COLLATE utf8_bin, + oldValue LONGTEXT NOT NULL COLLATE utf8_bin, + newValue LONGTEXT NOT NULL COLLATE utf8_bin, + metadata LONGTEXT NOT NULL COLLATE utf8_bin, + contentSource LONGTEXT NOT NULL COLLATE utf8_bin, + dateCreated INT UNSIGNED NOT NULL, + dateModified INT UNSIGNED NOT NULL, + UNIQUE KEY `key_phid` (phid), + KEY `key_object` (objectPHID) +) ENGINE=InnoDB DEFAULT CHARSET=utf8; + +CREATE TABLE {$NAMESPACE}_releeph.releeph_requesttransaction_comment ( + id INT UNSIGNED NOT NULL PRIMARY KEY AUTO_INCREMENT, + phid VARCHAR(64) NOT NULL COLLATE utf8_bin, + transactionPHID VARCHAR(64) COLLATE utf8_bin, + authorPHID VARCHAR(64) NOT NULL COLLATE utf8_bin, + viewPolicy VARCHAR(64) NOT NULL COLLATE utf8_bin, + editPolicy VARCHAR(64) NOT NULL COLLATE utf8_bin, + commentVersion INT UNSIGNED NOT NULL, + content LONGTEXT NOT NULL COLLATE utf8_bin, + contentSource LONGTEXT NOT NULL COLLATE utf8_bin, + isDeleted BOOL NOT NULL, + dateCreated INT UNSIGNED NOT NULL, + dateModified INT UNSIGNED NOT NULL, + + UNIQUE KEY `key_phid` (phid), + UNIQUE KEY `key_version` (transactionPHID, commentVersion) + +) ENGINE=InnoDB, COLLATE utf8_general_ci; diff --git a/resources/sql/patches/20130508.releephtransactionsmig.php b/resources/sql/patches/20130508.releephtransactionsmig.php new file mode 100644 index 0000000000..9aaf109ec2 --- /dev/null +++ b/resources/sql/patches/20130508.releephtransactionsmig.php @@ -0,0 +1,131 @@ +openTransaction(); +$table->beginReadLocking(); + +foreach (new LiskMigrationIterator($table) as $rq) { + printf("RQ%d:", $rq->getID()); + + $intents_cursor = array(); + $last_pick_status = null; + $last_commit_id = null; + + foreach ($rq->loadEvents() as $event) { + $author = $event->getActorPHID(); + $details = $event->getDetails(); + + $content_source = PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_UNKNOWN, + array('ReleephRequestEventID' => $event->getID())); + + $xaction = id(new ReleephRequestTransaction()) + ->setObjectPHID($rq->getPHID()) + ->setAuthorPHID($author) + ->setContentSource($content_source) + ->setDateCreated($event->getDateCreated()) + ->setDateModified($event->getDateModified()) + ->setViewPolicy(PhabricatorPolicies::POLICY_PUBLIC) + ->setEditPolicy($author); + + printf(" %s(#%d)", $event->getType(), $event->getID()); + + switch ($event->getType()) { + case ReleephRequestEvent::TYPE_COMMENT: + $xaction + ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) + ->save(); // to generate a PHID + $comment = id(new ReleephRequestTransactionComment()) + ->setAuthorPHID($author) + ->setTransactionPHID($xaction->getPHID()) + ->setViewPolicy(PhabricatorPolicies::POLICY_PUBLIC) + ->setEditPolicy($author) + ->setCommentVersion(1) + ->setContent($event->getComment()) + ->setContentSource($content_source) + ->save(); + $xaction + ->setCommentPHID($comment->getPHID()) + ->setCommentVersion(1); + break; + + case ReleephRequestEvent::TYPE_STATUS: + // Ignore STATUS events; these are legacy events that we no longer + // support anyway! + continue 2; + + case ReleephRequestEvent::TYPE_USER_INTENT: + $old_intent = idx($intents_cursor, $author); + $new_intent = $event->getDetail('newIntent'); + $xaction + ->setTransactionType(ReleephRequestTransaction::TYPE_USER_INTENT) + ->setMetadataValue('isAuthoritative', $event->getDetail('wasPusher')) + ->setOldValue($old_intent) + ->setNewValue($new_intent); + $intents_cursor[$author] = $new_intent; + break; + + case ReleephRequestEvent::TYPE_PICK_STATUS: + $new_pick_status = $event->getDetail('newPickStatus'); + $xaction + ->setTransactionType(ReleephRequestTransaction::TYPE_PICK_STATUS) + ->setOldValue($last_pick_status) + ->setNewValue($new_pick_status); + $last_pick_status = $new_pick_status; + break; + + case ReleephRequestEvent::TYPE_COMMIT: + $new_commit_id = $event->getDetail('newCommitIdentifier'); + $xaction + ->setTransactionType(ReleephRequestTransaction::TYPE_COMMIT) + ->setMetadataValue('action', $event->getDetail('action')) + ->setOldValue($last_commit_id) + ->setNewValue($new_commit_id); + $last_commit_id = $new_commit_id; + break; + + case ReleephRequestEvent::TYPE_DISCOVERY: + $xaction + ->setTransactionType(ReleephRequestTransaction::TYPE_DISCOVERY) + ->setNewValue($event->getDetail('newCommitPHID')); + break; + + case ReleephRequestEvent::TYPE_CREATE: + $xaction + ->setTransactionType(ReleephRequestTransaction::TYPE_REQUEST) + ->setOldValue(null) + ->setNewValue($rq->getRequestCommitPHID()); + break; + + case ReleephRequestEvent::TYPE_MANUAL_ACTION: + $xaction + ->setTransactionType( + ReleephRequestTransaction::TYPE_MANUAL_IN_BRANCH); + switch ($event->getDetail('action')) { + case 'pick': + $xaction->setNewValue(1); + break; + + case 'revert': + $xaction->setNewValue(0); + break; + } + break; + + default: + throw new Exception(sprintf( + "Unhandled ReleephRequestEvent type '%s' in RQ%d", + $event->getType(), + $rq->getID())); + } + + $xaction->save(); + } + echo("\n"); +} + +$table->endReadLocking(); +$table->saveTransaction(); +echo "Done.\n"; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index b8af5ff5ec..4a7a30b8ec 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1733,18 +1733,20 @@ phutil_register_library_map(array( 'ReleephReasonFieldSpecification' => 'applications/releeph/field/specification/ReleephReasonFieldSpecification.php', 'ReleephRequest' => 'applications/releeph/storage/ReleephRequest.php', 'ReleephRequestActionController' => 'applications/releeph/controller/request/ReleephRequestActionController.php', - 'ReleephRequestCreateController' => 'applications/releeph/controller/request/ReleephRequestCreateController.php', + 'ReleephRequestCommentController' => 'applications/releeph/controller/request/ReleephRequestCommentController.php', 'ReleephRequestDifferentialCreateController' => 'applications/releeph/controller/request/ReleephRequestDifferentialCreateController.php', 'ReleephRequestEditController' => 'applications/releeph/controller/request/ReleephRequestEditController.php', - 'ReleephRequestEditor' => 'applications/releeph/editor/ReleephRequestEditor.php', 'ReleephRequestEvent' => 'applications/releeph/storage/request/ReleephRequestEvent.php', - 'ReleephRequestEventListView' => 'applications/releeph/view/requestevent/ReleephRequestEventListView.php', 'ReleephRequestException' => 'applications/releeph/storage/request/exception/ReleephRequestException.php', 'ReleephRequestHeaderListView' => 'applications/releeph/view/request/header/ReleephRequestHeaderListView.php', 'ReleephRequestHeaderView' => 'applications/releeph/view/request/header/ReleephRequestHeaderView.php', 'ReleephRequestIntentsView' => 'applications/releeph/view/request/ReleephRequestIntentsView.php', - 'ReleephRequestMail' => 'applications/releeph/editor/mail/ReleephRequestMail.php', + 'ReleephRequestReplyHandler' => 'applications/releeph/mail/ReleephRequestReplyHandler.php', 'ReleephRequestStatusView' => 'applications/releeph/view/request/ReleephRequestStatusView.php', + 'ReleephRequestTransaction' => 'applications/releeph/storage/ReleephRequestTransaction.php', + 'ReleephRequestTransactionComment' => 'applications/releeph/storage/ReleephRequestTransactionComment.php', + 'ReleephRequestTransactionQuery' => 'applications/releeph/query/ReleephRequestTransactionQuery.php', + 'ReleephRequestTransactionalEditor' => 'applications/releeph/editor/ReleephRequestTransactionalEditor.php', 'ReleephRequestTypeaheadControl' => 'applications/releeph/view/request/ReleephRequestTypeaheadControl.php', 'ReleephRequestTypeaheadController' => 'applications/releeph/controller/request/ReleephRequestTypeaheadController.php', 'ReleephRequestViewController' => 'applications/releeph/controller/request/ReleephRequestViewController.php', @@ -3493,17 +3495,20 @@ phutil_register_library_map(array( 'ReleephReasonFieldSpecification' => 'ReleephFieldSpecification', 'ReleephRequest' => 'ReleephDAO', 'ReleephRequestActionController' => 'ReleephController', - 'ReleephRequestCreateController' => 'ReleephController', + 'ReleephRequestCommentController' => 'ReleephController', 'ReleephRequestDifferentialCreateController' => 'ReleephController', 'ReleephRequestEditController' => 'ReleephController', - 'ReleephRequestEditor' => 'PhabricatorEditor', 'ReleephRequestEvent' => 'ReleephDAO', - 'ReleephRequestEventListView' => 'AphrontView', 'ReleephRequestException' => 'Exception', 'ReleephRequestHeaderListView' => 'AphrontView', 'ReleephRequestHeaderView' => 'AphrontView', 'ReleephRequestIntentsView' => 'AphrontView', + 'ReleephRequestReplyHandler' => 'PhabricatorMailReplyHandler', 'ReleephRequestStatusView' => 'AphrontView', + 'ReleephRequestTransaction' => 'PhabricatorApplicationTransaction', + 'ReleephRequestTransactionComment' => 'PhabricatorApplicationTransactionComment', + 'ReleephRequestTransactionQuery' => 'PhabricatorApplicationTransactionQuery', + 'ReleephRequestTransactionalEditor' => 'PhabricatorApplicationTransactionEditor', 'ReleephRequestTypeaheadControl' => 'AphrontFormControl', 'ReleephRequestTypeaheadController' => 'PhabricatorTypeaheadDatasourceController', 'ReleephRequestViewController' => 'ReleephController', diff --git a/src/applications/releeph/application/PhabricatorApplicationReleeph.php b/src/applications/releeph/application/PhabricatorApplicationReleeph.php index 27a684b156..4154229be0 100644 --- a/src/applications/releeph/application/PhabricatorApplicationReleeph.php +++ b/src/applications/releeph/application/PhabricatorApplicationReleeph.php @@ -58,7 +58,7 @@ final class PhabricatorApplicationReleeph extends PhabricatorApplication { ), 'request/' => array( '(?P[1-9]\d*)/' => 'ReleephRequestViewController', - 'create/' => 'ReleephRequestCreateController', + 'create/' => 'ReleephRequestEditController', 'differentialcreate/' => array( 'D(?P[1-9]\d*)' => 'ReleephRequestDifferentialCreateController', @@ -69,13 +69,15 @@ final class PhabricatorApplicationReleeph extends PhabricatorApplication { 'ReleephRequestActionController', 'typeahead/' => 'ReleephRequestTypeaheadController', + 'comment/(?P[1-9]\d*)/' => + 'ReleephRequestCommentController', ), // Branch navigation made pretty, as it's the most common: '(?P[^/]+)/(?P[^/]+)/' => array( '' => 'ReleephBranchViewController', 'edit/' => 'ReleephBranchEditController', - 'request/' => 'ReleephRequestCreateController', + 'request/' => 'ReleephRequestEditController', '(?Pclose|re-open)/' => 'ReleephBranchAccessController', ), ) diff --git a/src/applications/releeph/conduit/ConduitAPI_releeph_request_Method.php b/src/applications/releeph/conduit/ConduitAPI_releeph_request_Method.php index 68b2a3010f..e0275e476d 100644 --- a/src/applications/releeph/conduit/ConduitAPI_releeph_request_Method.php +++ b/src/applications/releeph/conduit/ConduitAPI_releeph_request_Method.php @@ -27,6 +27,7 @@ final class ConduitAPI_releeph_request_Method } protected function execute(ConduitAPIRequest $request) { + $user = $request->getUser(); $branch_phid = $request->getValue('branchPHID'); $releeph_branch = id(new ReleephBranch()) ->loadOneWhere('phid = %s', $branch_phid); @@ -74,7 +75,7 @@ final class ConduitAPI_releeph_request_Method foreach ($requested_commits as $thing => $commit) { $phid = $commit->getPHID(); $handles = id(new PhabricatorObjectHandleData(array($phid))) - ->setViewer($request->getUser()) + ->setViewer($user) ->loadHandles(); $name = id($handles[$phid])->getName(); @@ -84,7 +85,25 @@ final class ConduitAPI_releeph_request_Method if ($existing_releeph_request) { $releeph_request = $existing_releeph_request; } else { - $releeph_request = new ReleephRequest(); + $releeph_request = id(new ReleephRequest()) + ->setRequestUserPHID($user->getPHID()) + ->setBranchID($releeph_branch->getID()) + ->setInBranch(0); + + $xactions = array(); + + $xactions[] = id(new ReleephRequestTransaction()) + ->setTransactionType(ReleephRequestTransaction::TYPE_REQUEST) + ->setNewValue($commit->getPHID()); + + $xactions[] = id(new ReleephRequestTransaction()) + ->setTransactionType(ReleephRequestTransaction::TYPE_USER_INTENT) + ->setMetadataValue('userPHID', $user->getPHID()) + ->setMetadataValue( + 'isAuthoritative', + $releeph_project->isAuthoritative($user)) + ->setNewValue(ReleephRequest::INTENT_WANT); + foreach ($fields as $field) { if (!$field->isEditable()) { continue; @@ -97,13 +116,20 @@ final class ConduitAPI_releeph_request_Method ->setErrorDescription($ex->getMessage()); } } - id(new ReleephRequestEditor($releeph_request)) - ->setActor($request->getUser()) - ->create($commit, $releeph_branch); + + $editor = id(new ReleephRequestTransactionalEditor()) + ->setActor($user) + ->setContinueOnNoEffect(true) + ->setContentSource( + PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_CONDUIT, + array())); + + $editor->applyTransactions($releeph_request, $xactions); } $releeph_branch->populateReleephRequestHandles( - $request->getUser(), + $user, array($releeph_request)); $rq_handles = $releeph_request->getHandles(); $requestor_phid = $releeph_request->getRequestUserPHID(); diff --git a/src/applications/releeph/conduit/work/ConduitAPI_releephwork_record_Method.php b/src/applications/releeph/conduit/work/ConduitAPI_releephwork_record_Method.php index 0b8bd3a1a8..626ddb41d1 100644 --- a/src/applications/releeph/conduit/work/ConduitAPI_releephwork_record_Method.php +++ b/src/applications/releeph/conduit/work/ConduitAPI_releephwork_record_Method.php @@ -7,8 +7,22 @@ final class ConduitAPI_releephwork_record_Method return self::METHOD_STATUS_UNSTABLE; } + /** + * Record that a request was committed locally, and is about to be pushed to + * the remote repository. + * + * This lets us mark a ReleephRequest as being in a branch in real time so + * that no one else tries to pick it. + * + * When the daemons discover this commit in the repository with + * DifferentialReleephRequestFieldSpecification, we'll be able to record the + * commit's PHID as well. That process is slow though, and we don't want to + * wait a whole minute before marking something as cleanly picked or + * reverted. + */ public function getMethodDescription() { - return "Wrapper to ReleephRequestEditor->recordSuccessfulCommit()."; + return "Record whether we committed a pick or revert ". + "to the upstream repository."; } public function defineParamTypes() { @@ -34,9 +48,22 @@ final class ConduitAPI_releephwork_record_Method $releeph_request = id(new ReleephRequest()) ->loadOneWhere('phid = %s', $request->getValue('requestPHID')); - id(new ReleephRequestEditor($releeph_request)) + $xactions = array(); + + $xactions[] = id(new ReleephRequestTransaction()) + ->setTransactionType(ReleephRequestTransaction::TYPE_COMMIT) + ->setMetadataValue('action', $action) + ->setNewValue($new_commit_id); + + $editor = id(new ReleephRequestTransactionalEditor()) ->setActor($request->getUser()) - ->recordSuccessfulCommit($action, $new_commit_id); + ->setContinueOnNoEffect(true) + ->setContentSource( + PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_CONDUIT, + array())); + + $editor->applyTransactions($releeph_request, $xactions); } } diff --git a/src/applications/releeph/conduit/work/ConduitAPI_releephwork_recordpickstatus_Method.php b/src/applications/releeph/conduit/work/ConduitAPI_releephwork_recordpickstatus_Method.php index 93fbcee068..9b515b8d39 100644 --- a/src/applications/releeph/conduit/work/ConduitAPI_releephwork_recordpickstatus_Method.php +++ b/src/applications/releeph/conduit/work/ConduitAPI_releephwork_recordpickstatus_Method.php @@ -8,7 +8,7 @@ final class ConduitAPI_releephwork_recordpickstatus_Method } public function getMethodDescription() { - return "Wrapper to ReleephRequestEditor->changePickStatus()."; + return "Record whether a pick or revert was successful or not."; } public function defineParamTypes() { @@ -55,9 +55,23 @@ final class ConduitAPI_releephwork_recordpickstatus_Method $releeph_request = id(new ReleephRequest()) ->loadOneWhere('phid = %s', $request->getValue('requestPHID')); - id(new ReleephRequestEditor($releeph_request)) + $editor = id(new ReleephRequestTransactionalEditor()) ->setActor($request->getUser()) - ->changePickStatus($pick_status, $dry_run, $details); + ->setContinueOnNoEffect(true) + ->setContentSource( + PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_CONDUIT, + array())); + + $xactions = array(); + + $xactions[] = id(new ReleephRequestTransaction()) + ->setTransactionType(ReleephRequestTransaction::TYPE_PICK_STATUS) + ->setMetadataValue('dryRun', $dry_run) + ->setMetadataValue('details', $details) + ->setNewValue($pick_status); + + $editor->applyTransactions($releeph_request, $xactions); } } diff --git a/src/applications/releeph/controller/request/ReleephRequestActionController.php b/src/applications/releeph/controller/request/ReleephRequestActionController.php index f7229cda2e..1c9daea570 100644 --- a/src/applications/releeph/controller/request/ReleephRequestActionController.php +++ b/src/applications/releeph/controller/request/ReleephRequestActionController.php @@ -12,6 +12,7 @@ final class ReleephRequestActionController extends ReleephController { public function processRequest() { $request = $this->getRequest(); + $releeph_project = $this->getReleephProject(); $releeph_branch = $this->getReleephBranch(); $releeph_request = $this->getReleephRequest(); @@ -24,8 +25,17 @@ final class ReleephRequestActionController extends ReleephController { $origin_uri = $releeph_request->loadReleephBranch()->getURI(); - $editor = id(new ReleephRequestEditor($releeph_request)) - ->setActor($user); + $editor = id(new ReleephRequestTransactionalEditor()) + ->setActor($user) + ->setContinueOnNoEffect(true) + ->setContentSource( + PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_WEB, + array( + 'ip' => $request->getRemoteAddr(), + ))); + + $xactions = array(); switch ($action) { case 'want': @@ -34,21 +44,49 @@ final class ReleephRequestActionController extends ReleephController { 'want' => ReleephRequest::INTENT_WANT, 'pass' => ReleephRequest::INTENT_PASS); $intent = $action_map[$action]; - $editor->changeUserIntent($user, $intent); + $xactions[] = id(new ReleephRequestTransaction()) + ->setTransactionType(ReleephRequestTransaction::TYPE_USER_INTENT) + ->setMetadataValue( + 'isAuthoritative', + $releeph_project->isAuthoritative($user)) + ->setNewValue($intent); break; case 'mark-manually-picked': - $editor->markManuallyActioned('pick'); - break; - case 'mark-manually-reverted': - $editor->markManuallyActioned('revert'); + if (!$releeph_project->isAuthoritative($user)) { + throw new Exception( + "Bug! Only authoritative users (pushers, or users in pusherless ". + "Releeph projects) can manually change a request's in-branch ". + "status!"); + } + + if ($action === 'mark-manually-picked') { + $in_branch = 1; + $intent = ReleephRequest::INTENT_WANT; + } else { + $in_branch = 0; + $intent = ReleephRequest::INTENT_PASS; + } + + $xactions[] = id(new ReleephRequestTransaction()) + ->setTransactionType(ReleephRequestTransaction::TYPE_USER_INTENT) + ->setMetadataValue('isManual', true) + ->setMetadataValue('isAuthoritative', true) + ->setNewValue($intent); + + $xactions[] = id(new ReleephRequestTransaction()) + ->setTransactionType(ReleephRequestTransaction::TYPE_MANUAL_IN_BRANCH) + ->setNewValue($in_branch); + break; default: throw new Exception("unknown or unimplemented action {$action}"); } + $editor->applyTransactions($releeph_request, $xactions); + // If we're adding a new user to userIntents, we'll have to re-populate // request handles to load that user's data. // diff --git a/src/applications/releeph/controller/request/ReleephRequestCommentController.php b/src/applications/releeph/controller/request/ReleephRequestCommentController.php new file mode 100644 index 0000000000..09e1f6186c --- /dev/null +++ b/src/applications/releeph/controller/request/ReleephRequestCommentController.php @@ -0,0 +1,63 @@ +getRequest(); + $user = $request->getUser(); + + $rq = $this->getReleephRequest(); + + if (!$request->isFormPost()) { + return new Aphront400Response(); + } + + $is_preview = $request->isPreviewRequest(); + $draft = PhabricatorDraft::buildFromRequest($request); + + $view_uri = $this->getApplicationURI('/RQ'.$rq->getID()); + + $xactions = array(); + $xactions[] = id(new ReleephRequestTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) + ->attachComment( + id(new ReleephRequestTransactionComment()) + ->setContent($request->getStr('comment'))); + + $editor = id(new ReleephRequestTransactionalEditor()) + ->setActor($user) + ->setContinueOnNoEffect($request->isContinueRequest()) + ->setContentSource( + PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_WEB, + array( + 'ip' => $request->getRemoteAddr(), + ))) + ->setIsPreview($is_preview); + + try { + $xactions = $editor->applyTransactions($rq, $xactions); + } catch (PhabricatorApplicationTransactionNoEffectException $ex) { + return id(new PhabricatorApplicationTransactionNoEffectResponse()) + ->setCancelURI($view_uri) + ->setException($ex); + } + + if ($draft) { + $draft->replaceOrDelete(); + } + + if ($request->isAjax()) { + return id(new PhabricatorApplicationTransactionResponse()) + ->setViewer($user) + ->setTransactions($xactions) + ->setIsPreview($is_preview) + ->setAnchorOffset($request->getStr('anchor')); + } else { + return id(new AphrontRedirectResponse()) + ->setURI($view_uri); + } + } + +} diff --git a/src/applications/releeph/controller/request/ReleephRequestCreateController.php b/src/applications/releeph/controller/request/ReleephRequestCreateController.php deleted file mode 100644 index c72a579a69..0000000000 --- a/src/applications/releeph/controller/request/ReleephRequestCreateController.php +++ /dev/null @@ -1,165 +0,0 @@ -getRequest(); - - // We arrived via /releeph/request/create/?branchID=$id - $releeph_branch_id = $request->getInt('branchID'); - if ($releeph_branch_id) { - $releeph_branch = id(new ReleephBranch())->load($releeph_branch_id); - } else { - // We arrived via /releeph/$project/$branch/request. - // - // If this throws an Exception, then somethind weird happened. - $releeph_branch = $this->getReleephBranch(); - } - - $releeph_project = $releeph_branch->loadReleephProject(); - $repo = $releeph_project->loadPhabricatorRepository(); - - $request_identifier = $request->getStr('requestIdentifierRaw'); - $e_request_identifier = true; - - $releeph_request = new ReleephRequest(); - - $errors = array(); - - $selector = $releeph_project->getReleephFieldSelector(); - $fields = $selector->getFieldSpecifications(); - foreach ($fields as $field) { - $field - ->setReleephProject($releeph_project) - ->setReleephBranch($releeph_branch) - ->setReleephRequest($releeph_request); - } - - if ($request->isFormPost()) { - foreach ($fields as $field) { - if ($field->isEditable()) { - try { - $field->setValueFromAphrontRequest($request); - } catch (ReleephFieldParseException $ex) { - $errors[] = $ex->getMessage(); - } - } - } - - $pr_commit = null; - $finder = id(new ReleephCommitFinder()) - ->setReleephProject($releeph_project); - try { - $pr_commit = $finder->fromPartial($request_identifier); - } catch (Exception $e) { - $e_request_identifier = pht('Invalid'); - $errors[] = - pht('Request %s is probably not a valid commit', $request_identifier); - $errors[] = $e->getMessage(); - } - - $pr_commit_data = null; - if (!$errors) { - $pr_commit_data = $pr_commit->loadCommitData(); - if (!$pr_commit_data) { - $e_request_identifier = pht('Not parsed yet'); - $errors[] = pht('The requested commit hasn\'t been parsed yet.'); - } - } - - if (!$errors) { - $existing = id(new ReleephRequest()) - ->loadOneWhere('requestCommitPHID = %s AND branchID = %d', - $pr_commit->getPHID(), $releeph_branch->getID()); - - if ($existing) { - return id(new AphrontRedirectResponse()) - ->setURI('/releeph/request/edit/'.$existing->getID(). - '?existing=1'); - } - - id(new ReleephRequestEditor($releeph_request)) - ->setActor($request->getUser()) - ->create($pr_commit, $releeph_branch); - - return id(new AphrontRedirectResponse()) - ->setURI($releeph_branch->getURI()); - } - } - - $error_view = null; - if ($errors) { - $error_view = new AphrontErrorView(); - $error_view->setErrors($errors); - $error_view->setTitle(pht('Form Errors')); - } - - // For the typeahead - $branch_cut_point = id(new PhabricatorRepositoryCommit()) - ->loadOneWhere( - 'phid = %s', - $releeph_branch->getCutPointCommitPHID()); - - // Build the form - $form = id(new AphrontFormView()) - ->setUser($request->getUser()); - - $origin = null; - $diff_rev_id = $request->getStr('D'); - if ($diff_rev_id) { - $diff_rev = id(new DifferentialRevision())->load($diff_rev_id); - $origin = '/D'.$diff_rev->getID(); - $title = sprintf( - 'D%d: %s', - $diff_rev_id, - $diff_rev->getTitle()); - $form - ->addHiddenInput('requestIdentifierRaw', 'D'.$diff_rev_id) - ->appendChild( - id(new AphrontFormStaticControl()) - ->setLabel(pht('Diff')) - ->setValue($title)); - } else { - $origin = $releeph_branch->getURI(); - $form->appendChild( - id(new ReleephRequestTypeaheadControl()) - ->setName('requestIdentifierRaw') - ->setLabel('Commit ID') - ->setRepo($repo) - ->setValue($request_identifier) - ->setError($e_request_identifier) - ->setStartTime($branch_cut_point->getEpoch()) - ->setCaption( - pht('Start typing to autocomplete on commit title, '. - 'or give a Phabricator commit identifier like rFOO1234'))); - } - - // Fields - foreach ($fields as $field) { - if ($field->isEditable()) { - $control = $field->renderEditControl($request); - $form->appendChild($control); - } - } - - $form - ->appendChild( - id(new AphrontFormSubmitControl()) - ->addCancelButton($origin) - ->setValue(pht('Request'))); - - $panel = id(new AphrontPanelView()) - ->setHeader( - pht('Request for %s', - $releeph_branch->getDisplayNameWithDetail())) - ->setWidth(AphrontPanelView::WIDTH_FORM) - ->appendChild($form); - - return $this->buildStandardPageResponse( - array($error_view, $panel), - array('title' => pht('Request Pick'))); - } - -} diff --git a/src/applications/releeph/controller/request/ReleephRequestDifferentialCreateController.php b/src/applications/releeph/controller/request/ReleephRequestDifferentialCreateController.php index 791c31f1e5..63ba5a9cea 100644 --- a/src/applications/releeph/controller/request/ReleephRequestDifferentialCreateController.php +++ b/src/applications/releeph/controller/request/ReleephRequestDifferentialCreateController.php @@ -91,8 +91,8 @@ final class ReleephRequestDifferentialCreateController } private function buildReleephRequestURI(ReleephBranch $branch) { - return id(new PhutilURI('/releeph/request/create/')) - ->setQueryParam('branchID', $branch->getID()) + $uri = $branch->getURI('request/'); + return id(new PhutilURI($uri)) ->setQueryParam('D', $this->revision->getID()); } diff --git a/src/applications/releeph/controller/request/ReleephRequestEditController.php b/src/applications/releeph/controller/request/ReleephRequestEditController.php index 9ccbb23315..a458aab3f9 100644 --- a/src/applications/releeph/controller/request/ReleephRequestEditController.php +++ b/src/applications/releeph/controller/request/ReleephRequestEditController.php @@ -2,39 +2,43 @@ final class ReleephRequestEditController extends ReleephController { + private $id; + + public function willProcessRequest(array $data) { + $this->id = idx($data, 'requestID'); + parent::willProcessRequest($data); + } + public function processRequest() { $request = $this->getRequest(); + $user = $request->getUser(); - $releeph_branch = $this->getReleephBranch(); - $releeph_request = $this->getReleephRequest(); + $releeph_project = $this->getReleephProject(); + $releeph_branch = $this->getReleephBranch(); - $releeph_branch->populateReleephRequestHandles( - $request->getUser(), array($releeph_request)); + $request_identifier = $request->getStr('requestIdentifierRaw'); + $e_request_identifier = true; - $phids = array(); - $phids[] = $releeph_request->getRequestCommitPHID(); - $phids[] = $releeph_request->getRequestUserPHID(); + // Load the RQ we're editing, or create a new one + if ($this->id) { + $rq = id(new ReleephRequest())->load($this->id); + $is_edit = true; + } else { + $is_edit = false; + $rq = id(new ReleephRequest()) + ->setRequestUserPHID($user->getPHID()) + ->setBranchID($releeph_branch->getID()) + ->setInBranch(0); + } - $handles = id(new PhabricatorObjectHandleData($phids)) - ->setViewer($request->getUser()) - ->loadHandles(); - - $age_string = phabricator_format_relative_time( - time() - $releeph_request->getDateCreated()); - - // Warn the user if we see this - $notice_view = null; - if ($request->getInt('existing')) { - $notice_messages = array( - 'You are editing an existing pick request!', - hsprintf( - "Requested %s ago by %s", - $age_string, - $handles[$releeph_request->getRequestUserPHID()]->renderLink()) - ); - $notice_view = id(new AphrontErrorView()) - ->setSeverity(AphrontErrorView::SEVERITY_NOTICE) - ->setErrors($notice_messages); + // Load all the ReleephFieldSpecifications + $selector = $this->getReleephProject()->getReleephFieldSelector(); + $fields = $selector->getFieldSpecifications(); + foreach ($fields as $field) { + $field + ->setReleephProject($releeph_project) + ->setReleephBranch($releeph_branch) + ->setReleephRequest($rq); } // epriestley: Is it common to pass around a referer URL to @@ -43,43 +47,134 @@ final class ReleephRequestEditController extends ReleephController { // rather than the full URL. switch ($request->getStr('origin')) { case 'request': - $origin_uri = '/RQ'.$releeph_request->getID(); + $origin_uri = '/RQ'.$rq->getID(); break; case 'branch': default: - $origin_uri = $releeph_request->loadReleephBranch()->getURI(); + $origin_uri = $releeph_branch->getURI(); break; } + // Make edits $errors = array(); - - $selector = $this->getReleephProject()->getReleephFieldSelector(); - $fields = $selector->getFieldSpecifications(); - foreach ($fields as $field) { - $field - ->setReleephProject($this->getReleephProject()) - ->setReleephBranch($this->getReleephBranch()) - ->setReleephRequest($this->getReleephRequest()); - } - if ($request->isFormPost()) { - foreach ($fields as $field) { - if ($field->isEditable()) { + $xactions = array(); + + // The commit-identifier being requested... + if (!$is_edit) { + if ($request_identifier === + ReleephRequestTypeaheadControl::PLACEHOLDER) { + + $errors[] = "No commit ID was provided."; + $e_request_identifier = 'Required'; + } else { + $pr_commit = null; + $finder = id(new ReleephCommitFinder()) + ->setReleephProject($releeph_project); try { - $field->setValueFromAphrontRequest($request); - } catch (ReleephFieldParseException $ex) { - $errors[] = $ex->getMessage(); + $pr_commit = $finder->fromPartial($request_identifier); + } catch (Exception $e) { + $e_request_identifier = 'Invalid'; + $errors[] = + "Request {$request_identifier} is probably not a valid commit"; + $errors[] = $e->getMessage(); + } + + $pr_commit_data = null; + if (!$errors) { + $pr_commit_data = $pr_commit->loadCommitData(); + if (!$pr_commit_data) { + $e_request_identifier = 'Not parsed yet'; + $errors[] = "The requested commit hasn't been parsed yet."; + } + } + } + + if (!$errors) { + $existing = id(new ReleephRequest()) + ->loadOneWhere('requestCommitPHID = %s AND branchID = %d', + $pr_commit->getPHID(), $releeph_branch->getID()); + if ($existing) { + return id(new AphrontRedirectResponse()) + ->setURI('/releeph/request/edit/'.$existing->getID(). + '?existing=1'); + } + + $xactions[] = id(new ReleephRequestTransaction()) + ->setTransactionType(ReleephRequestTransaction::TYPE_REQUEST) + ->setNewValue($pr_commit->getPHID()); + + $xactions[] = id(new ReleephRequestTransaction()) + ->setTransactionType(ReleephRequestTransaction::TYPE_USER_INTENT) + // To help hide these implicit intents... + ->setMetadataValue('isRQCreate', true) + ->setMetadataValue('userPHID', $user->getPHID()) + ->setMetadataValue( + 'isAuthoritative', + $releeph_project->isAuthoritative($user)) + ->setNewValue(ReleephRequest::INTENT_WANT); + } + } + + if (!$errors) { + foreach ($fields as $field) { + if ($field->isEditable()) { + try { + $data = $request->getRequestData(); + $value = idx($data, $field->getRequiredStorageKey()); + $field->validate($value); + $xactions[] = id(new ReleephRequestTransaction()) + ->setTransactionType(ReleephRequestTransaction::TYPE_EDIT_FIELD) + ->setMetadataValue('fieldClass', get_class($field)) + ->setNewValue($value); + } catch (ReleephFieldParseException $ex) { + $errors[] = $ex->getMessage(); + } } } } if (!$errors) { - $releeph_request->save(); + $editor = id(new ReleephRequestTransactionalEditor()) + ->setActor($user) + ->setContinueOnNoEffect(true) + ->setContentSource( + PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_WEB, + array( + 'ip' => $request->getRemoteAddr(), + ))); + $editor->applyTransactions($rq, $xactions); return id(new AphrontRedirectResponse())->setURI($origin_uri); } } + $releeph_branch->populateReleephRequestHandles($user, array($rq)); + $handles = $rq->getHandles(); + + $age_string = ''; + if ($is_edit) { + $age_string = phabricator_format_relative_time( + time() - $rq->getDateCreated()) . ' ago'; + } + + // Warn the user if we've been redirected here because we tried to + // re-request something. + $notice_view = null; + if ($request->getInt('existing')) { + $notice_messages = array( + 'You are editing an existing pick request!', + hsprintf( + "Requested %s by %s", + $age_string, + $handles[$rq->getRequestUserPHID()]->renderLink()) + ); + $notice_view = id(new AphrontErrorView()) + ->setSeverity(AphrontErrorView::SEVERITY_NOTICE) + ->setErrors($notice_messages); + } + /** * Build the rest of the page */ @@ -91,19 +186,58 @@ final class ReleephRequestEditController extends ReleephController { } $form = id(new AphrontFormView()) - ->setUser($request->getUser()) - ->appendChild( - id(new AphrontFormMarkupControl()) - ->setLabel('Original Commit') - ->setValue( - $handles[$releeph_request->getRequestCommitPHID()]->renderLink())) - ->appendChild( - id(new AphrontFormMarkupControl()) - ->setLabel('Requestor') - ->setValue(hsprintf( - '%s %s ago', - $handles[$releeph_request->getRequestUserPHID()]->renderLink(), - $age_string))); + ->setUser($request->getUser()); + + if ($is_edit) { + $form + ->appendChild( + id(new AphrontFormMarkupControl()) + ->setLabel('Original Commit') + ->setValue( + $handles[$rq->getRequestCommitPHID()]->renderLink())) + ->appendChild( + id(new AphrontFormMarkupControl()) + ->setLabel('Requestor') + ->setValue(hsprintf( + '%s %s', + $handles[$rq->getRequestUserPHID()]->renderLink(), + $age_string))); + } else { + $origin = null; + $diff_rev_id = $request->getStr('D'); + if ($diff_rev_id) { + $diff_rev = id(new DifferentialRevision())->load($diff_rev_id); + $origin = '/D'.$diff_rev->getID(); + $title = sprintf( + 'D%d: %s', + $diff_rev_id, + $diff_rev->getTitle()); + $form + ->addHiddenInput('requestIdentifierRaw', 'D'.$diff_rev_id) + ->appendChild( + id(new AphrontFormStaticControl()) + ->setLabel('Diff') + ->setValue($title)); + } else { + $origin = $releeph_branch->getURI(); + $repo = $releeph_project->loadPhabricatorRepository(); + $branch_cut_point = id(new PhabricatorRepositoryCommit()) + ->loadOneWhere( + 'phid = %s', + $releeph_branch->getCutPointCommitPHID()); + $form->appendChild( + id(new ReleephRequestTypeaheadControl()) + ->setName('requestIdentifierRaw') + ->setLabel('Commit ID') + ->setRepo($repo) + ->setValue($request_identifier) + ->setError($e_request_identifier) + ->setStartTime($branch_cut_point->getEpoch()) + ->setCaption( + 'Start typing to autocomplete on commit title, '. + 'or give a Phabricator commit identifier like rFOO1234')); + } + } // Fields foreach ($fields as $field) { @@ -113,19 +247,27 @@ final class ReleephRequestEditController extends ReleephController { } } + if ($is_edit) { + $title = pht('Edit Releeph Request'); + $submit_name = pht('Save'); + } else { + $title = pht('Create Releeph Request'); + $submit_name = pht('Create'); + } + $form ->appendChild( id(new AphrontFormSubmitControl()) ->addCancelButton($origin_uri, 'Cancel') - ->setValue('Save')); + ->setValue($submit_name)); $panel = id(new AphrontPanelView()) - ->setHeader('Edit Pick Request') + ->setHeader($title) ->setWidth(AphrontPanelView::WIDTH_FORM) ->appendChild($form); return $this->buildStandardPageResponse( array($notice_view, $error_view, $panel), - array('title', 'Edit Pick Request')); + array('title', $title)); } } diff --git a/src/applications/releeph/controller/request/ReleephRequestViewController.php b/src/applications/releeph/controller/request/ReleephRequestViewController.php index f0cd2661fe..7390ce1bd5 100644 --- a/src/applications/releeph/controller/request/ReleephRequestViewController.php +++ b/src/applications/releeph/controller/request/ReleephRequestViewController.php @@ -29,41 +29,43 @@ final class ReleephRequestViewController extends ReleephController { ->setReloadOnStateChange(true) ->setOriginType('request'); - $events = $releeph_request->loadEvents(); - $phids = array_mergev(mpull($events, 'extractPHIDs')); - $handles = id(new PhabricatorObjectHandleData($phids)) - ->setViewer($request->getUser()) - ->loadHandles(); + $user = $request->getUser(); - $rq_event_list_view = - id(new ReleephRequestEventListView()) - ->setUser($request->getUser()) - ->setEvents($events) - ->setHandles($handles); + $engine = id(new PhabricatorMarkupEngine()) + ->setViewer($user); - // Handle comment submit - $origin_uri = '/RQ'.$releeph_request->getID(); - if ($request->isFormPost()) { - id(new ReleephRequestEditor($releeph_request)) - ->setActor($request->getUser()) - ->addComment($request->getStr('comment')); - return id(new AphrontRedirectResponse())->setURI($origin_uri); + $xactions = id(new ReleephRequestTransactionQuery()) + ->setViewer($user) + ->withObjectPHIDs(array($releeph_request->getPHID())) + ->execute(); + + foreach ($xactions as $xaction) { + if ($xaction->getComment()) { + $engine->addObject( + $xaction->getComment(), + PhabricatorApplicationTransactionComment::MARKUP_FIELD_COMMENT); + } } + $engine->process(); - $form = id(new AphrontFormView()) + $timeline = id(new PhabricatorApplicationTransactionView()) ->setUser($request->getUser()) - ->appendChild( - id(new AphrontFormTextAreaControl()) - ->setName('comment')) - ->appendChild( - id(new AphrontFormSubmitControl()) - ->addCancelButton($origin_uri, 'Cancel') - ->setValue("Submit")); + ->setTransactions($xactions) + ->setMarkupEngine($engine); - $rq_comment_form = id(new AphrontPanelView()) - ->setHeader('Add a comment') - ->setWidth(AphrontPanelView::WIDTH_FULL) - ->appendChild($form); + $add_comment_header = id(new PhabricatorHeaderView()) + ->setHeader('Plea or yield'); + + $draft = PhabricatorDraft::newFromUserAndKey( + $user, + $releeph_request->getPHID()); + + $add_comment_form = id(new PhabricatorApplicationTransactionCommentView()) + ->setUser($user) + ->setDraft($draft) + ->setAction($this->getApplicationURI( + '/request/comment/'.$releeph_request->getID().'/')) + ->setSubmitButtonName('Comment'); $title = hsprintf("RQ%d: %s", $releeph_request->getID(), @@ -88,8 +90,9 @@ final class ReleephRequestViewController extends ReleephController { $crumbs, array( $rq_view, - $rq_event_list_view, - $rq_comment_form + $timeline, + $add_comment_header, + $add_comment_form, ) ), array( diff --git a/src/applications/releeph/differential/DifferentialReleephRequestFieldSpecification.php b/src/applications/releeph/differential/DifferentialReleephRequestFieldSpecification.php index 840241747a..e568646797 100644 --- a/src/applications/releeph/differential/DifferentialReleephRequestFieldSpecification.php +++ b/src/applications/releeph/differential/DifferentialReleephRequestFieldSpecification.php @@ -268,9 +268,26 @@ final class DifferentialReleephRequestFieldSpecification $actor = id(new PhabricatorUser()) ->loadOneWhere('phid = %s', $actor_phid); - id(new ReleephRequestEditor($releeph_request)) + $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) - ->discoverCommit($action, $commit, $data); + ->setContinueOnNoEffect(true) + ->setContentSource( + PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_UNKNOWN, + array())); + + $editor->applyTransactions($releeph_request, $xactions); } } diff --git a/src/applications/releeph/editor/ReleephRequestEditor.php b/src/applications/releeph/editor/ReleephRequestEditor.php deleted file mode 100644 index 9b6e31b310..0000000000 --- a/src/applications/releeph/editor/ReleephRequestEditor.php +++ /dev/null @@ -1,402 +0,0 @@ -releephRequest = $rq; - } - - public function setSilentUpdate($silent) { - $this->silentUpdate = $silent; - return $this; - } - - -/* -( ReleephRequest edit methods )---------------------------------------- */ - - /** - * Request a PhabricatorRepositoryCommit to be committed to the given - * ReleephBranch. - */ - public function create(PhabricatorRepositoryCommit $commit, - ReleephBranch $branch) { - - // We can't use newEvent() / commit() abstractions, so do what those - // helpers do manually. - $requestor = $this->requireActor(); - - $rq = $this->releephRequest; - $rq->openTransaction(); - - $rq - ->setBranchID($branch->getID()) - ->setRequestCommitPHID($commit->getPHID()) - ->setInBranch(0) - ->setRequestUserPHID($requestor->getPHID()) - ->setUserIntent($requestor, ReleephRequest::INTENT_WANT) - ->save(); - - $event = id(new ReleephRequestEvent()) - ->setType(ReleephRequestEvent::TYPE_CREATE) - ->setActorPHID($requestor->getPHID()) - ->setStatusBefore(null) - ->setStatusAfter($rq->getStatus()) - ->setReleephRequestID($rq->getID()) - ->setDetail('commitPHID', $commit->getPHID()) - ->save(); - - $rq->saveTransaction(); - - // Mail - if (!$this->silentUpdate) { - $project = $this->releephRequest->loadReleephProject(); - $mail = id(new ReleephRequestMail()) - ->setReleephRequest($this->releephRequest) - ->setReleephProject($project) - ->setEvents(array($event)) - ->setSenderAndRecipientPHID($requestor->getPHID()) - ->addTos(ReleephRequestMail::ENT_ALL_PUSHERS) - ->addCCs(ReleephRequestMail::ENT_REQUESTOR) - ->send(); - } - } - - /** - * Record whether the PhabricatorUser wants or passes on this request. - */ - public function changeUserIntent(PhabricatorUser $user, $intent) { - $project = $this->releephRequest->loadReleephProject(); - $is_pusher = $project->isPusher($user); - - $event = $this->newEvent() - ->setType(ReleephRequestEvent::TYPE_USER_INTENT) - ->setDetail('userPHID', $user->getPHID()) - ->setDetail('wasPusher', $is_pusher) - ->setDetail('newIntent', $intent); - - $this->releephRequest - ->setUserIntent($user, $intent); - - $this->commit(); - - // Mail if this is 'interesting' - if (!$this->silentUpdate && - $event->getStatusBefore() != $event->getStatusAfter()) { - - $project = $this->releephRequest->loadReleephProject(); - $mail = id(new ReleephRequestMail()) - ->setReleephRequest($this->releephRequest) - ->setReleephProject($project) - ->setEvents(array($event)) - ->setSenderAndRecipientPHID($this->requireActor()->getPHID()) - ->addTos(ReleephRequestMail::ENT_REQUESTOR) - ->addCCs(ReleephRequestMail::ENT_INTERESTED_PUSHERS) - ->send(); - } - } - - /** - * Record the results of someone trying to pick or revert a request in their - * local repository, to give advance warning that something doesn't pick or - * revert cleanly. - */ - public function changePickStatus($pick_status, $dry_run, $details) { - $event = $this->newEvent() - ->setType(ReleephRequestEvent::TYPE_PICK_STATUS) - ->setDetail('newPickStatus', $pick_status) - ->setDetail('commitDetails', $details); - $this->releephRequest->setPickStatus($pick_status); - $this->commit(); - - // Failures should generate an email - if (!$this->silentUpdate && - !$dry_run && - ($pick_status == ReleephRequest::PICK_FAILED || - $pick_status == ReleephRequest::REVERT_FAILED)) { - - $project = $this->releephRequest->loadReleephProject(); - $mail = id(new ReleephRequestMail()) - ->setReleephRequest($this->releephRequest) - ->setReleephProject($project) - ->setEvents(array($event)) - ->setSenderAndRecipientPHID($this->requireActor()->getPHID()) - ->addTos(ReleephRequestMail::ENT_REQUESTOR) - ->addCCs(ReleephRequestMail::ENT_ACTORS) - ->addCCs(ReleephRequestMail::ENT_INTERESTED_PUSHERS) - ->send(); - } - } - - /** - * Record that a request was committed locally, and is about to be pushed to - * the remote repository. - * - * This lets us mark a ReleephRequest as being in a branch in real time so - * that no one else tries to pick it. - * - * When the daemons discover this commit in the repository with - * DifferentialReleephRequestFieldSpecification, we'll be able to recrod the - * commit's PHID as well. That process is slow though, and - * we don't want to wait a whole minute before marking something as cleanly - * picked or reverted. - */ - public function recordSuccessfulCommit($action, $new_commit_id) { - $table = $this->releephRequest; - $table->openTransaction(); - - $actor = $this->requireActor(); - - $event = id(new ReleephRequestEvent()) - ->setReleephRequestID($this->releephRequest->getID()) - ->setActorPHID($actor->getPHID()) - ->setType(ReleephRequestEvent::TYPE_COMMIT) - ->setDetail('action', $action) - ->setDetail('newCommitIdentifier', $new_commit_id) - ->save(); - - switch ($action) { - case 'pick': - $this->releephRequest - ->setInBranch(1) - ->setPickStatus(ReleephRequest::PICK_OK) - ->setCommitIdentifier($new_commit_id) - ->setCommitPHID(null) - ->save(); - break; - - case 'revert': - $this->releephRequest - ->setInBranch(0) - ->setPickStatus(ReleephRequest::REVERT_OK) - ->setCommitIdentifier(null) - ->setCommitPHID(null) - ->save(); - break; - - default: - $table->killTransaction(); - throw new Exception("Unknown action {$action}!"); - break; - } - - $table->saveTransaction(); - - // Don't spam people about local commits -- we'll do that with - // discoverCommit() instead! - } - - /** - * Mark this request as picked or reverted based on discovering it in the - * branch. We have a PhabricatorRepositoryCommit, so we're able to - * setCommitPHID on the ReleephRequest (unlike recordSuccessfulCommit()). - */ - public function discoverCommit( - $action, - PhabricatorRepositoryCommit $commit, - PhabricatorRepositoryCommitData $data) { - - $table = $this->releephRequest; - $table->openTransaction(); - $table->beginWriteLocking(); - - $past_events = id(new ReleephRequestEvent())->loadAllWhere( - 'releephRequestID = %d AND type = %s', - $this->releephRequest->getID(), - ReleephRequestEvent::TYPE_DISCOVERY); - - foreach ($past_events as $past_event) { - if ($past_event->getDetail('newCommitIdentifier') - == $commit->getCommitIdentifier()) { - - // Avoid re-discovery if reparsing! - $table->endWriteLocking(); - $table->killTransaction(); - return; - } - } - - $actor = $this->requireActor(); - - $event = id(new ReleephRequestEvent()) - ->setReleephRequestID($this->releephRequest->getID()) - ->setActorPHID($actor->getPHID()) - ->setType(ReleephRequestEvent::TYPE_DISCOVERY) - ->setDateCreated($commit->getEpoch()) - ->setDetail('action', $action) - ->setDetail('newCommitIdentifier', $commit->getCommitIdentifier()) - ->setDetail('newCommitPHID', $commit->getPHID()) - ->setDetail('authorPHID', $data->getCommitDetail('authorPHID')) - ->setDetail('committerPHID', $data->getCommitDetail('committerPHID')) - ->save(); - - switch ($action) { - case 'pick': - $this->releephRequest - ->setInBranch(1) - ->setPickStatus(ReleephRequest::PICK_OK) - ->setCommitIdentifier($commit->getCommitIdentifier()) - ->setCommitPHID($commit->getPHID()) - ->save(); - break; - - case 'revert': - $this->releephRequest - ->setInBranch(0) - ->setPickStatus(ReleephRequest::REVERT_OK) - ->setCommitIdentifier(null) - ->setCommitPHID(null) - ->save(); - break; - - default: - $table->killTransaction(); - throw new Exception("Unknown action {$action}!"); - break; - } - - $table->endWriteLocking(); - $table->saveTransaction(); - - // Mail - if (!$this->silentUpdate) { - $project = $this->releephRequest->loadReleephProject(); - $mail = id(new ReleephRequestMail()) - ->setReleephRequest($this->releephRequest) - ->setReleephProject($project) - ->setEvents(array($event)) - ->setSenderAndRecipientPHID($this->requireActor()->getPHID()) - ->addTos(ReleephRequestMail::ENT_REQUESTOR) - ->addCCs(ReleephRequestMail::ENT_ACTORS) - ->addCCs(ReleephRequestMail::ENT_INTERESTED_PUSHERS) - ->send(); - } - } - - public function addComment($comment) { - $event = $this->newEvent() - ->setType(ReleephRequestEvent::TYPE_COMMENT) - ->setDetail('comment', $comment); - $this->commit(); - - // Mail - if (!$this->silentUpdate) { - $project = $this->releephRequest->loadReleephProject(); - $mail = id(new ReleephRequestMail()) - ->setReleephRequest($this->releephRequest) - ->setReleephProject($project) - ->setEvents(array($event)) - ->setSenderAndRecipientPHID($this->requireActor()->getPHID()) - ->addTos(ReleephRequestMail::ENT_REQUESTOR) - ->addCCs(ReleephRequestMail::ENT_ACTORS) - ->addCCs(ReleephRequestMail::ENT_INTERESTED_PUSHERS) - ->send(); - } - } - - public function markManuallyActioned($action) { - $event = $this->newEvent() - ->setType(ReleephRequestEvent::TYPE_MANUAL_ACTION) - ->setDetail('action', $action); - - $actor = $this->requireActor(); - $project = $this->releephRequest->loadReleephProject(); - $requestor_phid = $this->releephRequest->getRequestUserPHID(); - if (!$project->isPusher($actor) && - $actor->getPHID() !== $requestor_phid) { - - throw new Exception( - "Only pushers or requestors can mark requests as ". - "manually picked or reverted!"); - } - - switch ($action) { - case 'pick': - $in_branch = true; - $intent = ReleephRequest::INTENT_WANT; - break; - - case 'revert': - $in_branch = false; - $intent = ReleephRequest::INTENT_PASS; - break; - - default: - throw new Exception("Unknown action {$action}!"); - break; - } - - $this->releephRequest - ->setInBranch((int)$in_branch) - ->setUserIntent($this->getActor(), $intent); - - $this->commit(); - - // Mail - if (!$this->silentUpdate) { - $project = $this->releephRequest->loadReleephProject(); - $mail = id(new ReleephRequestMail()) - ->setReleephRequest($this->releephRequest) - ->setReleephProject($project) - ->setEvents(array($event)) - ->setSenderAndRecipientPHID($this->requireActor()->getPHID()) - ->addTos(ReleephRequestMail::ENT_REQUESTOR) - ->addCCs(ReleephRequestMail::ENT_INTERESTED_PUSHERS) - ->send(); - } - } - -/* -( Implementation )----------------------------------------------------- */ - - /** - * Create and return a new ReleephRequestEvent bound to the editor's - * ReleephRequest, inside a transaction. - * - * When you call commit(), the event and this editor's ReleephRequest (along - * with any changes you made to the ReleephRequest) are saved and the - * transaction committed. - */ - private function newEvent() { - $actor = $this->requireActor(); - - if ($this->event) { - throw new Exception("You have already called newEvent()!"); - } - $rq = $this->releephRequest; - $rq->openTransaction(); - - $this->event = id(new ReleephRequestEvent()) - ->setReleephRequestID($rq->getID()) - ->setActorPHID($actor->getPHID()) - ->setStatusBefore($rq->getStatus()); - - return $this->event; - } - - private function commit() { - if (!$this->event) { - throw new Exception("You must call newEvent first!"); - } - $rq = $this->releephRequest; - $this->event - ->setStatusAfter($rq->getStatus()) - ->save(); - $rq->save(); - $rq->saveTransaction(); - $this->event = null; - } - -} diff --git a/src/applications/releeph/editor/ReleephRequestTransactionalEditor.php b/src/applications/releeph/editor/ReleephRequestTransactionalEditor.php new file mode 100644 index 0000000000..e175b4db5a --- /dev/null +++ b/src/applications/releeph/editor/ReleephRequestTransactionalEditor.php @@ -0,0 +1,300 @@ +getTransactionType()) { + case ReleephRequestTransaction::TYPE_REQUEST: + return $object->getRequestCommitPHID(); + + case ReleephRequestTransaction::TYPE_EDIT_FIELD: + $field = newv($xaction->getMetadataValue('fieldClass'), array()); + $value = $field->setReleephRequest($object)->getValue(); + return $value; + + case ReleephRequestTransaction::TYPE_USER_INTENT: + $user_phid = $xaction->getAuthorPHID(); + return idx($object->getUserIntents(), $user_phid); + + case ReleephRequestTransaction::TYPE_PICK_STATUS: + return (int)$object->getPickStatus(); + break; + + case ReleephRequestTransaction::TYPE_COMMIT: + return $object->getCommitIdentifier(); + + case ReleephRequestTransaction::TYPE_DISCOVERY: + return $object->getCommitPHID(); + + case ReleephRequestTransaction::TYPE_MANUAL_IN_BRANCH: + return $object->getInBranch(); + } + } + + public function getCustomTransactionNewValue( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + + switch ($xaction->getTransactionType()) { + case ReleephRequestTransaction::TYPE_REQUEST: + case ReleephRequestTransaction::TYPE_USER_INTENT: + case ReleephRequestTransaction::TYPE_EDIT_FIELD: + case ReleephRequestTransaction::TYPE_PICK_STATUS: + case ReleephRequestTransaction::TYPE_COMMIT: + case ReleephRequestTransaction::TYPE_DISCOVERY: + case ReleephRequestTransaction::TYPE_MANUAL_IN_BRANCH: + return $xaction->getNewValue(); + } + } + + public function applyCustomInternalTransaction( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + + $new = $xaction->getNewValue(); + + switch ($xaction->getTransactionType()) { + case ReleephRequestTransaction::TYPE_REQUEST: + $object->setRequestCommitPHID($new); + break; + + case ReleephRequestTransaction::TYPE_USER_INTENT: + $user_phid = $xaction->getAuthorPHID(); + $intents = $object->getUserIntents(); + $intents[$user_phid] = $new; + $object->setUserIntents($intents); + break; + + case ReleephRequestTransaction::TYPE_EDIT_FIELD: + $field = newv($xaction->getMetadataValue('fieldClass'), array()); + $field + ->setReleephRequest($object) + ->setValue($new); + break; + + case ReleephRequestTransaction::TYPE_PICK_STATUS: + $object->setPickStatus($new); + break; + + case ReleephRequestTransaction::TYPE_COMMIT: + $this->setInBranchFromAction($object, $xaction); + $object->setCommitIdentifier($new); + break; + + case ReleephRequestTransaction::TYPE_DISCOVERY: + $this->setInBranchFromAction($object, $xaction); + $object->setCommitPHID($new); + break; + + case ReleephRequestTransaction::TYPE_MANUAL_IN_BRANCH: + $object->setInBranch((int) $new); + break; + } + } + + protected function applyCustomExternalTransaction( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + + return; + } + + protected function filterTransactions( + PhabricatorLiskDAO $object, + array $xactions) { + + // Remove TYPE_DISCOVERY xactions that are the result of a reparse. + $previously_discovered_commits = array(); + $discovery_xactions = idx( + mgroup($xactions, 'getTransactionType'), + ReleephRequestTransaction::TYPE_DISCOVERY); + if ($discovery_xactions) { + $previous_xactions = id(new ReleephRequestTransactionQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withObjectPHIDs(array($object->getPHID())) + ->execute(); + + foreach ($previous_xactions as $xaction) { + if ($xaction->getTransactionType() === + ReleephRequestTransaction::TYPE_DISCOVERY) { + + $commit_phid = $xaction->getNewValue(); + $previously_discovered_commits[$commit_phid] = true; + } + } + } + + foreach ($xactions as $key => $xaction) { + if ($xaction->getTransactionType() === + ReleephRequestTransaction::TYPE_DISCOVERY && + idx($previously_discovered_commits, $xaction->getNewValue())) { + + unset($xactions[$key]); + } + } + + return parent::filterTransactions($object, $xactions); + } + + protected function supportsMail() { + return true; + } + + protected function sendMail( + PhabricatorLiskDAO $object, + array $xactions) { + + // Avoid sending emails that only talk about commit discovery. + $types = array_unique(mpull($xactions, 'getTransactionType')); + if ($types === array(ReleephRequestTransaction::TYPE_DISCOVERY)) { + return null; + } + + // Don't email people when we discover that something picks or reverts OK. + if ($types === array(ReleephRequestTransaction::TYPE_PICK_STATUS)) { + if (!mfilter($xactions, 'isBoringPickStatus', true /* negate */)) { + // If we effectively call "isInterestingPickStatus" and get nothing... + return null; + } + } + + return parent::sendMail($object, $xactions); + } + + protected function buildReplyHandler(PhabricatorLiskDAO $object) { + return id(new ReleephRequestReplyHandler()) + ->setActor($this->getActor()) + ->setMailReceiver($object); + } + + protected function getMailSubjectPrefix() { + return '[Releeph]'; + } + + protected function buildMailTemplate(PhabricatorLiskDAO $object) { + $id = $object->getID(); + $phid = $object->getPHID(); + $title = $object->getSummaryForDisplay(); + return id(new PhabricatorMetaMTAMail()) + ->setSubject("RQ{$id}: {$title}") + ->addHeader('Thread-Topic', "RQ{$id}: {$phid}"); + } + + protected function getMailTo(PhabricatorLiskDAO $object) { + $to_phids = array(); + + $releeph_project = $object->loadReleephProject(); + foreach ($releeph_project->getPushers() as $phid) { + $to_phids[] = $phid; + } + + foreach ($object->getUserIntents() as $phid => $intent) { + $to_phids[] = $phid; + } + + return $to_phids; + } + + protected function getMailCC(PhabricatorLiskDAO $object) { + return array(); + } + + protected function buildMailBody( + PhabricatorLiskDAO $object, + array $xactions) { + + $body = parent::buildMailBody($object, $xactions); + + $rq = $object; + $releeph_branch = $rq->loadReleephBranch(); + $releeph_project = $releeph_branch->loadReleephProject(); + + /** + * If any of the events we are emailing about were about a pick failure + * (and/or a revert failure?), include pick failure instructions. + */ + $has_pick_failure = false; + foreach ($xactions as $xaction) { + if ($xaction->getTransactionType() === + ReleephRequestTransaction::TYPE_PICK_STATUS && + $xaction->getNewValue() === ReleephRequest::PICK_FAILED) { + + $has_pick_failure = true; + break; + } + } + if ($has_pick_failure) { + $instructions = $releeph_project->getDetail('pick_failure_instructions'); + if ($instructions) { + $body->addTextSection( + pht('PICK FAILURE INSTRUCTIONS'), + $instructions); + } + } + + $name = sprintf("RQ%s: %s", $rq->getID(), $rq->getSummaryForDisplay()); + $body->addTextSection( + pht('RELEEPH REQUEST'), + $name."\n". + PhabricatorEnv::getProductionURI('/RQ'.$rq->getID())); + + $project_and_branch = sprintf( + '%s - %s', + $releeph_project->getName(), + $releeph_branch->getDisplayNameWithDetail()); + + $body->addTextSection( + pht('RELEEPH BRANCH'), + $project_and_branch."\n". + $releeph_branch->getURI()); + + return $body; + } + + private function setInBranchFromAction( + ReleephRequest $rq, + ReleephRequestTransaction $xaction) { + + $action = $xaction->getMetadataValue('action'); + switch ($action) { + case 'pick': + $rq->setInBranch(1); + break; + + case 'revert': + $rq->setInBranch(0); + break; + + default: + $id = $rq->getID(); + $type = $xaction->getTransactionType(); + $new = $xaction->getNewValue(); + phlog( + "Unknown discovery action '{$action}' ". + "for xaction of type {$type} ". + "with new value {$new} ". + "mentioning RQ{$id}!"); + break; + } + } + +} diff --git a/src/applications/releeph/editor/mail/ReleephRequestMail.php b/src/applications/releeph/editor/mail/ReleephRequestMail.php deleted file mode 100644 index a06f6f3342..0000000000 --- a/src/applications/releeph/editor/mail/ReleephRequestMail.php +++ /dev/null @@ -1,213 +0,0 @@ -releephRequest = $rq; - return $this; - } - - public function setReleephProject(ReleephProject $rp) { - $this->releephProject = $rp; - return $this; - } - - public function setEvents(array $events) { - assert_instances_of($events, 'ReleephRequestEvent'); - $this->events = $events; - return $this; - } - - public function setSenderAndRecipientPHID($sender_phid) { - $this->sender = $sender_phid; - $this->tos[] = $sender_phid; - return $this; - } - - public function addTos($entity) { - $this->tos = array_merge( - $this->tos, - $this->getEntityPHIDs($entity)); - return $this; - } - - public function addCcs($entity) { - $this->ccs = array_merge( - $this->tos, - $this->getEntityPHIDs($entity)); - return $this; - } - - public function send() { - $this->buildMail()->save(); - } - - public function buildMail() { - return id(new PhabricatorMetaMTAMail()) - ->setSubject($this->renderSubject()) - ->setBody($this->buildBody()->render()) - ->setFrom($this->sender) - ->addTos($this->tos) - ->addCCs($this->ccs); - } - - private function getEntityPHIDs($entity) { - $phids = array(); - switch ($entity) { - // The requestor - case self::ENT_REQUESTOR: - $phids[] = $this->releephRequest->getRequestUserPHID(); - break; - - // People on the original diff - case self::ENT_DIFF: - $commit = $this->releephRequest->loadPhabricatorRepositoryCommit(); - $commit_data = $commit->loadCommitData(); - if ($commit_data) { - $phids[] = $commit_data->getCommitDetail('reviewerPHID'); - $phids[] = $commit_data->getCommitDetail('authorPHID'); - } - break; - - // All pushers for this project - case self::ENT_ALL_PUSHERS: - $phids = array_merge( - $phids, - $this->releephProject->getPushers()); - break; - - // Pushers who have explicitly wanted or passed on this request - case self::ENT_INTERESTED_PUSHERS: - $all_pushers = $this->releephProject->getPushers(); - $intents = $this->releephRequest->getUserIntents(); - foreach ($all_pushers as $pusher) { - if (idx($intents, $pusher)) { - $phids[] = $pusher; - } - } - break; - - // Anyone who created our list of events - case self::ENT_ACTORS: - $phids = array_merge( - $phids, - mpull($this->events, 'getActorPHID')); - break; - - default: - throw new Exception( - "Unknown entity type {$entity}!"); - break; - } - - return array_filter($phids); - } - - private function buildBody() { - $body = new PhabricatorMetaMTAMailBody(); - $rq = $this->releephRequest; - - // Events and comments - $phids = array( - $rq->getPHID(), - ); - foreach ($this->events as $event) { - $phids = array_merge($phids, $event->extractPHIDs()); - } - $handles = id(new PhabricatorObjectHandleData($phids)) - // By the time we're generating email, we can assume that whichever - // entitties are receving the email are authorized to see the loaded - // handles! - ->setViewer(PhabricatorUser::getOmnipotentUser()) - ->loadHandles(); - - $raw_events = id(new ReleephRequestEventListView()) - ->setUser(PhabricatorUser::getOmnipotentUser()) - ->setHandles($handles) - ->setEvents($this->events) - ->renderForEmail(); - - $body->addRawSection($raw_events); - - $project = $rq->loadReleephProject(); - $branch = $rq->loadReleephBranch(); - - /** - * If any of the events we are emailing about were TYPE_PICK_STATUS where - * the newPickStatus was a pick failure (and/or a revert failure?), include - * pick failure instructions. - */ - $pick_failure_events = array(); - foreach ($this->events as $event) { - if ($event->getType() == ReleephRequestEvent::TYPE_PICK_STATUS && - $event->getDetail('newPickStatus') == ReleephRequest::PICK_FAILED) { - - $pick_failure_events[] = $event; - } - } - - if ($pick_failure_events) { - $instructions = $project->getDetail('pick_failure_instructions'); - if ($instructions) { - $body->addTextSection('PICK FAILURE INSTRUCTIONS', $instructions); - } - } - - // Common stuff at the end - $body->addTextSection( - 'RELEEPH REQUEST', - $handles[$rq->getPHID()]->getFullName()."\n". - PhabricatorEnv::getProductionURI('/RQ'.$rq->getID())); - - $project_and_branch = sprintf( - '%s - %s', - $project->getName(), - $branch->getDisplayNameWithDetail()); - - $body->addTextSection( - 'RELEEPH BRANCH', - $project_and_branch."\n". - $branch->getURI()); - - // But verbose stuff at the *very* end! - foreach ($pick_failure_events as $event) { - $failure_details = $event->getDetail('commitDetails'); - if ($failure_details) { - $body->addRawSection('PICK FAILURE DETAILS'); - foreach ($failure_details as $heading => $data) { - $body->addTextSection($heading, $data); - } - } - } - - return $body; - } - - private function renderSubject() { - $rq = $this->releephRequest; - $id = $rq->getID(); - $summary = $rq->getSummaryForDisplay(); - return "RQ{$id}: {$summary}"; - } - -} diff --git a/src/applications/releeph/field/specification/ReleephFieldSpecification.php b/src/applications/releeph/field/specification/ReleephFieldSpecification.php index a93d258210..74cce8f9a9 100644 --- a/src/applications/releeph/field/specification/ReleephFieldSpecification.php +++ b/src/applications/releeph/field/specification/ReleephFieldSpecification.php @@ -11,6 +11,22 @@ abstract class ReleephFieldSpecification return null; } + public function getRequiredStorageKey() { + $key = $this->getStorageKey(); + if ($key === null) { + throw new ReleephFieldSpecificationIncompleteException($this); + } + if (strpos($key, '.') !== false) { + /** + * Storage keys are reused for form controls, and periods in form control + * names break HTML forms. + */ + throw new Exception( + "You can't use '.' in storage keys!"); + } + return $key; + } + final public function isEditable() { return $this->getStorageKey() !== null; } @@ -37,6 +53,17 @@ abstract class ReleephFieldSpecification return; } + /** + * Turn values as they are stored in a ReleephRequest into a text that can be + * rendered as a transactions old/new values. + */ + public function normalizeForTransactionView( + PhabricatorApplicationTransaction $xaction, + $value) { + + return $value; + } + /* -( Header View )-------------------------------------------------------- */ @@ -60,13 +87,6 @@ abstract class ReleephFieldSpecification throw new ReleephFieldSpecificationIncompleteException($this); } - public function setValueFromAphrontRequest(AphrontRequest $request) { - $data = $request->getRequestData(); - $value = idx($data, $this->getRequiredStorageKey()); - $this->validate($value); - $this->setValue($value); - } - /* -( Conduit )------------------------------------------------------------ */ @@ -300,22 +320,6 @@ abstract class ReleephFieldSpecification /* -( Implementation )----------------------------------------------------- */ - protected function getRequiredStorageKey() { - $key = $this->getStorageKey(); - if ($key === null) { - throw new ReleephFieldSpecificationIncompleteException($this); - } - if (strpos($key, '.') !== false) { - /** - * Storage keys are reused for form controls, and periods in form control - * names break HTML forms. - */ - throw new Exception( - "You can't use '.' in storage keys!"); - } - return $key; - } - /** * The "hook" functions ##appendSelectControlsHook()## and * ##selectReleephRequestsHook()## are used with ##hasSelectablePHIDs()##, to diff --git a/src/applications/releeph/mail/ReleephRequestReplyHandler.php b/src/applications/releeph/mail/ReleephRequestReplyHandler.php new file mode 100644 index 0000000000..f402fa0711 --- /dev/null +++ b/src/applications/releeph/mail/ReleephRequestReplyHandler.php @@ -0,0 +1,55 @@ +getDefaultPrivateReplyHandlerEmailAddress($handle, 'RERQ'); + } + + public function getPublicReplyHandlerEmailAddress() { + return $this->getDefaultPublicReplyHandlerEmailAddress('RERQ'); + } + + public function getReplyHandlerInstructions() { + if ($this->supportsReplies()) { + return pht('Reply to comment.'); + } else { + return null; + } + } + + protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) { + $rq = $this->getMailReceiver(); + $user = $this->getActor(); + + $content_source = PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_EMAIL, + array( + 'id' => $mail->getID(), + )); + + $editor = id(new ReleephRequestTransactionalEditor()) + ->setActor($user) + ->setContentSource($content_source) + ->setParentMessageID($mail->getMessageID()); + + $body = $mail->getCleanTextBody(); + + $xactions = array(); + $xactions[] = id(new ReleephRequestTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) + ->attachComment($body); + + $editor->applyTransactions($rq, $xactions); + + return $rq; + } + +} diff --git a/src/applications/releeph/query/ReleephRequestTransactionQuery.php b/src/applications/releeph/query/ReleephRequestTransactionQuery.php new file mode 100644 index 0000000000..f049f9556d --- /dev/null +++ b/src/applications/releeph/query/ReleephRequestTransactionQuery.php @@ -0,0 +1,10 @@ +getTransactionType()) { + default; + break; + } + return parent::hasChangeDetails(); + } + + public function getRequiredHandlePHIDs() { + $phids = parent::getRequiredHandlePHIDs(); + $phids[] = $this->getObjectPHID(); + + $new = $this->getNewValue(); + + switch ($this->getTransactionType()) { + case ReleephRequestTransaction::TYPE_REQUEST: + case ReleephRequestTransaction::TYPE_DISCOVERY: + $phids[] = $new; + break; + + case ReleephRequestTransaction::TYPE_EDIT_FIELD: + self::searchForPHIDs($this->getOldValue(), $phids); + self::searchForPHIDs($this->getNewValue(), $phids); + break; + } + + return $phids; + } + + public function getTitle() { + $author_phid = $this->getAuthorPHID(); + $object_phid = $this->getObjectPHID(); + + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + switch ($this->getTransactionType()) { + case ReleephRequestTransaction::TYPE_REQUEST: + return pht( + '%s requested %s', + $this->renderHandleLink($author_phid), + $this->renderHandleLink($new)); + break; + + case ReleephRequestTransaction::TYPE_USER_INTENT: + return $this->getIntentTitle(); + break; + + case ReleephRequestTransaction::TYPE_EDIT_FIELD: + $field = newv($this->getMetadataValue('fieldClass'), array()); + $name = $field->getName(); + + $markup = $name; + if ($this->getRenderingTarget() === + PhabricatorApplicationTransaction::TARGET_HTML) { + + $markup = hsprintf('%s', $name); + } + + return pht( + '%s changed the %s to "%s"', + $this->renderHandleLink($author_phid), + $markup, + $field->normalizeForTransactionView($this, $new)); + break; + + case ReleephRequestTransaction::TYPE_PICK_STATUS: + switch ($new) { + case ReleephRequest::PICK_OK: + return pht('%s found this request picks without error', + $this->renderHandleLink($author_phid)); + + case ReleephRequest::REVERT_OK: + return pht('%s found this request reverts without error', + $this->renderHandleLink($author_phid)); + + case ReleephRequest::PICK_FAILED: + return pht("%s couldn't pick this request", + $this->renderHandleLink($author_phid)); + + case ReleephRequest::REVERT_FAILED: + return pht("%s couldn't revert this request", + $this->renderHandleLink($author_phid)); + } + break; + + case ReleephRequestTransaction::TYPE_COMMIT: + $action_type = $this->getMetadataValue('action'); + switch ($action_type) { + case 'pick': + return pht( + '%s picked this request and committed the result upstream', + $this->renderHandleLink($author_phid)); + break; + + case 'revert': + return pht( + '%s reverted this request and committed the result upstream', + $this->renderHandleLink($author_phid)); + break; + } + break; + + case ReleephRequestTransaction::TYPE_MANUAL_IN_BRANCH: + $action = $new ? pht('picked') : pht('reverted'); + return pht( + '%s marked this request as manually %s', + $this->renderHandleLink($author_phid), + $action); + break; + + case ReleephRequestTransaction::TYPE_DISCOVERY: + return pht('%s discovered this commit as %s', + $this->renderHandleLink($author_phid), + $this->renderHandleLink($new)); + break; + + default: + return parent::getTitle(); + break; + } + } + + public function getActionStrength() { + return parent::getActionStrength(); + } + + public function getActionName() { + switch ($this->getTransactionType()) { + case self::TYPE_REQUEST: + return pht('Requested'); + + case self::TYPE_COMMIT: + $action_type = $this->getMetadataValue('action'); + switch ($action_type) { + case 'pick': + return pht('Picked'); + + case 'revert': + return pht('Reverted'); + } + } + + return parent::getActionName(); + } + + public function getColor() { + $new = $this->getNewValue(); + + switch ($this->getTransactionType()) { + case ReleephRequestTransaction::TYPE_USER_INTENT: + switch ($new) { + case ReleephRequest::INTENT_WANT: + return PhabricatorTransactions::COLOR_GREEN; + case ReleephRequest::INTENT_PASS: + return PhabricatorTransactions::COLOR_RED; + } + } + return parent::getColor(); + } + + private static function searchForPHIDs($thing, array &$phids) { + /** + * To implement something like getRequiredHandlePHIDs() in a + * ReleephFieldSpecification, we'd have to provide the field with its + * ReleephRequest (so that it could load the PHIDs from the + * ReleephRequest's storage, and return them.) + * + * We don't have fields initialized with their ReleephRequests, but we can + * make a good guess at what handles will be needed for rendering the field + * in this transaction by inspecting the old and new values. + */ + if (!is_array($thing)) { + $thing = array($thing); + } + + foreach ($thing as $value) { + if (phid_get_type($value) !== + PhabricatorPHIDConstants::PHID_TYPE_UNKNOWN) { + + $phids[] = $value; + } + } + } + + private function getIntentTitle() { + $author_phid = $this->getAuthorPHID(); + $object_phid = $this->getObjectPHID(); + + $new = $this->getNewValue(); + $is_pusher = $this->getMetadataValue('isPusher'); + + switch ($new) { + case ReleephRequest::INTENT_WANT: + if ($is_pusher) { + return pht( + '%s approved this request', + $this->renderHandleLink($author_phid)); + } else { + return pht( + '%s wanted this request', + $this->renderHandleLink($author_phid)); + } + + case ReleephRequest::INTENT_PASS: + if ($is_pusher) { + return pht( + '%s rejected this request', + $this->renderHandleLink($author_phid)); + } else { + return pht( + '%s passed on this request', + $this->renderHandleLink($author_phid)); + } + } + } + + public function shouldHide() { + $type = $this->getTransactionType(); + + if ($type === ReleephRequestTransaction::TYPE_USER_INTENT && + $this->getMetadataValue('isRQCreate')) { + + return true; + } + + if ($this->isBoringPickStatus()) { + return true; + } + + // ReleephSummaryFieldSpecification is usually blank when an RQ is created, + // creating a transaction change from null to "". Hide these! + if ($type === ReleephRequestTransaction::TYPE_EDIT_FIELD) { + if ($this->getOldValue() === null && $this->getNewValue() === "") { + return true; + } + } + return parent::shouldHide(); + } + + public function isBoringPickStatus() { + $type = $this->getTransactionType(); + if ($type === ReleephRequestTransaction::TYPE_PICK_STATUS) { + $new = $this->getNewValue(); + if ($new === ReleephRequest::PICK_OK || + $new === ReleephRequest::REVERT_OK) { + + return true; + } + } + return false; + } + +} diff --git a/src/applications/releeph/storage/ReleephRequestTransactionComment.php b/src/applications/releeph/storage/ReleephRequestTransactionComment.php new file mode 100644 index 0000000000..71844b7367 --- /dev/null +++ b/src/applications/releeph/storage/ReleephRequestTransactionComment.php @@ -0,0 +1,10 @@ + $id, 'src' => '/releeph/request/typeahead/', - 'placeholder' => 'Type a commit id or first line of commit message...', + 'placeholder' => self::PLACEHOLDER, 'value' => $this->getValue(), 'aux' => array( 'repo' => $this->repo->getID(), diff --git a/src/applications/releeph/view/requestevent/ReleephRequestEventListView.php b/src/applications/releeph/view/requestevent/ReleephRequestEventListView.php deleted file mode 100644 index ab0bca7d57..0000000000 --- a/src/applications/releeph/view/requestevent/ReleephRequestEventListView.php +++ /dev/null @@ -1,266 +0,0 @@ -events = $events; - return $this; - } - - public function setHandles(array $handles) { - assert_instances_of($handles, 'PhabricatorObjectHandle'); - $this->handles = $handles; - return $this; - } - - public function render() { - $views = array(); - - $discovered_commits = array(); - foreach ($this->events as $event) { - $commit_id = $event->getDetail('newCommitIdentifier'); - switch ($event->getType()) { - case ReleephRequestEvent::TYPE_DISCOVERY: - $discovered_commits[$commit_id] = true; - break; - } - } - - $markup_engine = PhabricatorMarkupEngine::newDifferentialMarkupEngine(); - $markup_engine->setConfig('viewer', $this->getUser()); - - foreach ($this->events as $event) { - $description = $this->describeEvent($event); - if (!$description) { - continue; - } - - if ($event->getType() === ReleephRequestEvent::TYPE_COMMIT) { - $commit_id = $event->getDetail('newCommitIdentifier'); - if (idx($discovered_commits, $commit_id)) { - continue; - } - } - - $actor_handle = $this->handles[$event->getActorPHID()]; - $description = $this->describeEvent($event); - $action = phutil_tag( - 'div', - array(), - array( - $actor_handle->renderLink(), - ' ', - $description)); - - $view = id(new PhabricatorTransactionView()) - ->setUser($this->user) - ->setImageURI($actor_handle->getImageURI()) - ->setEpoch($event->getDateCreated()) - ->setActions(array($action)) - ->addClass($this->getTransactionClass($event)); - - $comment = $this->getEventComment($event); - if ($comment) { - $markup = phutil_tag( - 'div', - array( - 'class' => 'phabricator-remarkup', - ), - phutil_safe_html( - $markup_engine->markupText($comment))); - $view->appendChild($markup); - } - - $views[] = $view; - } - - return phutil_tag( - 'div', - array( - 'class' => 'releeph-request-event-list', - ), - $views); - } - - public function renderForEmail() { - $items = array(); - foreach ($this->events as $event) { - $description = $this->describeEvent($event); - if (!$description) { - continue; - } - $actor = $this->handles[$event->getActorPHID()]->getName(); - $items[] = $actor.' '.$description; - - $comment = $this->getEventComment($event); - if ($comment) { - $items[] = preg_replace('/^/m', ' ', $comment); - } - } - - return implode("\n\n", $items); - } - - private function describeEvent(ReleephRequestEvent $event) { - $type = $event->getType(); - - switch ($type) { - case ReleephRequestEvent::TYPE_CREATE: - return "created this request."; - break; - - case ReleephRequestEvent::TYPE_STATUS: - $status = $event->getStatusAfter(); - return sprintf( - "updated status to %s.", - ReleephRequest::getStatusDescriptionFor($status)); - break; - - case ReleephRequestEvent::TYPE_USER_INTENT: - $intent = $event->getDetail('newIntent'); - $was_pusher = $event->getDetail('wasPusher'); - if ($intent == ReleephRequest::INTENT_WANT) { - if ($was_pusher) { - $verb = "approved"; - } else { - $verb = "wanted"; - } - } else { - if ($was_pusher) { - $verb = "rejected"; - } else { - $verb = "passed on"; - } - } - return "{$verb} this request."; - break; - - case ReleephRequestEvent::TYPE_PICK_STATUS: - $pick_status = $event->getDetail('newPickStatus'); - switch ($pick_status) { - case ReleephRequest::PICK_FAILED: - return "found a conflict when picking."; - break; - - case ReleephRequest::REVERT_FAILED: - return "found a conflict when reverting."; - break; - - case ReleephRequest::PICK_OK: - case ReleephRequest::REVERT_OK: - // (nothing) - break; - - default: - return "changed pick-status to {$pick_status}."; - break; - } - break; - - case ReleephRequestEvent::TYPE_MANUAL_ACTION: - $action = $event->getDetail('action'); - return "claimed to have manually {$action}ed this request."; - break; - - case ReleephRequestEvent::TYPE_COMMIT: - $action = $event->getDetail('action'); - if ($action) { - return "{$action}ed this request."; - } else { - return "did something with this request."; - } - break; - - case ReleephRequestEvent::TYPE_DISCOVERY: - $action = $event->getDetail('action'); - if ($action) { - return "{$action}ed this request."; - } else { - // It's unlikely we'll have action-less TYPE_DISCOVERY events, but I - // used this during testing and I guess it's a useful safety net. - return "discovered this request in the branch."; - } - break; - - case ReleephRequestEvent::TYPE_COMMENT: - return "commented on this request."; - break; - - default: - return "did event of type {$type}."; - break; - } - } - - private function getEventComment(ReleephRequestEvent $event) { - switch ($event->getType()) { - case ReleephRequestEvent::TYPE_CREATE: - $commit_phid = $event->getDetail('commitPHID'); - return sprintf( - "Commit %s was requested.", - $this->handles[$commit_phid]->getName()); - break; - - case ReleephRequestEvent::TYPE_STATUS: - case ReleephRequestEvent::TYPE_USER_INTENT: - case ReleephRequestEvent::TYPE_PICK_STATUS: - case ReleephRequestEvent::TYPE_MANUAL_ACTION: - // no comment! - break; - - case ReleephRequestEvent::TYPE_COMMIT: - return sprintf( - "Closed by commit %s.", - $event->getDetail('newCommitIdentifier')); - break; - - case ReleephRequestEvent::TYPE_DISCOVERY: - $author_phid = $event->getDetail('authorPHID'); - $commit_phid = $event->getDetail('newCommitPHID'); - if ($author_phid && $author_phid != $event->getActorPHID()) { - return sprintf( - "Closed by commit %s (with author set to @%s).", - $this->handles[$commit_phid]->getName(), - $this->handles[$author_phid]->getName()); - } else { - return sprintf( - 'Closed by commit %s.', - $this->handles[$commit_phid]->getName()); - } - break; - - case ReleephRequestEvent::TYPE_COMMENT: - return $event->getComment(); - break; - } - } - - private function getTransactionClass($event) { - switch ($event->getType()) { - case ReleephRequestEvent::TYPE_COMMIT: - case ReleephRequestEvent::TYPE_DISCOVERY: - $action = $event->getDetail('action'); - if ($action == 'pick') { - return 'releeph-border-color-picked'; - } else { - return 'releeph-border-color-abandoned'; - } - break; - - case ReleephRequestEvent::TYPE_COMMENT: - return 'releeph-border-color-comment'; - break; - - default: - $status_after = $event->getStatusAfter(); - $class_suffix = ReleephRequest::getStatusClassSuffixFor($status_after); - return ' releeph-border-color-'.$class_suffix; - break; - } - } - -} diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index bcfef70f16..519ecb9e8c 100644 --- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php @@ -1286,6 +1286,14 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'sql', 'name' => $this->getPatchPath('20130508.search_namedquery.sql'), ), + '20130508.releephtransactions.sql' => array( + 'type' => 'sql', + 'name' => $this->getPatchPath('20130508.releephtransactions.sql'), + ), + '20130508.releephtransactionsmig.php' => array( + 'type' => 'php', + 'name' => $this->getPatchPath('20130508.releephtransactionsmig.php'), + ), ); } }