diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 2bfa0b596a..527a3ffaa3 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -174,6 +174,7 @@ phutil_register_library_map(array( 'DifferentialFieldSpecification' => 'applications/differential/field/specification/base', 'DifferentialFieldSpecificationIncompleteException' => 'applications/differential/field/exception/incomplete', 'DifferentialFieldValidationException' => 'applications/differential/field/exception/validation', + 'DifferentialGitSVNIDFieldSpecification' => 'applications/differential/field/specification/gitsvnid', 'DifferentialHostFieldSpecification' => 'applications/differential/field/specification/host', 'DifferentialHunk' => 'applications/differential/storage/hunk', 'DifferentialInlineComment' => 'applications/differential/storage/inlinecomment', @@ -191,6 +192,7 @@ phutil_register_library_map(array( 'DifferentialReplyHandler' => 'applications/differential/replyhandler', 'DifferentialRevertPlanFieldSpecification' => 'applications/differential/field/specification/revertplan', 'DifferentialReviewRequestMail' => 'applications/differential/mail/reviewrequest', + 'DifferentialReviewedByFieldSpecification' => 'applications/differential/field/specification/reviewedby', 'DifferentialReviewersFieldSpecification' => 'applications/differential/field/specification/reviewers', 'DifferentialRevision' => 'applications/differential/storage/revision', 'DifferentialRevisionCommentListView' => 'applications/differential/view/revisioncommentlist', @@ -200,6 +202,7 @@ phutil_register_library_map(array( 'DifferentialRevisionDetailView' => 'applications/differential/view/revisiondetail', 'DifferentialRevisionEditController' => 'applications/differential/controller/revisionedit', 'DifferentialRevisionEditor' => 'applications/differential/editor/revision', + 'DifferentialRevisionIDFieldSpecification' => 'applications/differential/field/specification/revisionid', 'DifferentialRevisionListController' => 'applications/differential/controller/revisionlist', 'DifferentialRevisionListData' => 'applications/differential/data/revisionlist', 'DifferentialRevisionStatus' => 'applications/differential/constants/revisionstatus', @@ -822,6 +825,7 @@ phutil_register_library_map(array( 'DifferentialDiffViewController' => 'DifferentialController', 'DifferentialExceptionMail' => 'DifferentialMail', 'DifferentialExportPatchFieldSpecification' => 'DifferentialFieldSpecification', + 'DifferentialGitSVNIDFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialHostFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialHunk' => 'DifferentialDAO', 'DifferentialInlineComment' => 'DifferentialDAO', @@ -837,12 +841,14 @@ phutil_register_library_map(array( 'DifferentialReplyHandler' => 'PhabricatorMailReplyHandler', 'DifferentialRevertPlanFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialReviewRequestMail' => 'DifferentialMail', + 'DifferentialReviewedByFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialReviewersFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialRevision' => 'DifferentialDAO', 'DifferentialRevisionCommentListView' => 'AphrontView', 'DifferentialRevisionCommentView' => 'AphrontView', 'DifferentialRevisionDetailView' => 'AphrontView', 'DifferentialRevisionEditController' => 'DifferentialController', + 'DifferentialRevisionIDFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialRevisionListController' => 'DifferentialController', 'DifferentialRevisionStatusFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialRevisionUpdateHistoryView' => 'AphrontView', diff --git a/src/applications/conduit/method/differential/updaterevision/ConduitAPI_differential_updaterevision_Method.php b/src/applications/conduit/method/differential/updaterevision/ConduitAPI_differential_updaterevision_Method.php index 633761b4ed..428d5ee97b 100644 --- a/src/applications/conduit/method/differential/updaterevision/ConduitAPI_differential_updaterevision_Method.php +++ b/src/applications/conduit/method/differential/updaterevision/ConduitAPI_differential_updaterevision_Method.php @@ -54,6 +54,9 @@ class ConduitAPI_differential_updaterevision_Method extends ConduitAPIMethod { } $revision = id(new DifferentialRevision())->load($request->getValue('id')); + if (!$revision) { + throw new ConduitException('ERR_BAD_REVISION'); + } if ($request->getUser()->getPHID() !== $revision->getAuthorPHID()) { throw new ConduitException('ERR_WRONG_USER'); diff --git a/src/applications/differential/editor/revision/DifferentialRevisionEditor.php b/src/applications/differential/editor/revision/DifferentialRevisionEditor.php index b5bb7075b4..c71f66d55c 100644 --- a/src/applications/differential/editor/revision/DifferentialRevisionEditor.php +++ b/src/applications/differential/editor/revision/DifferentialRevisionEditor.php @@ -70,15 +70,36 @@ class DifferentialRevisionEditor { $revision = $this->revision; - $revision->setTitle((string)$fields['title']); - $revision->setSummary((string)$fields['summary']); - $revision->setTestPlan((string)$fields['testPlan']); - $revision->setBlameRevision((string)$fields['blameRevision']); - $revision->setRevertPlan((string)$fields['revertPlan']); + $aux_fields = DifferentialFieldSelector::newSelector() + ->getFieldSpecifications(); - $this->setReviewers($fields['reviewerPHIDs']); - $this->setCCPHIDs($fields['ccPHIDs']); - $this->setTasks($fields['tasks']); + foreach ($aux_fields as $key => $aux_field) { + $aux_field->setRevision($revision); + if (!$aux_field->shouldAppearOnCommitMessage()) { + unset($aux_fields[$key]); + } + } + + $aux_fields = mpull($aux_fields, null, 'getCommitMessageKey'); + + foreach ($fields as $field => $value) { + + if ($field == 'tasks') { + // TODO: Deprecate once this can be fully supported with custom fields. + // This is just to prevent a backcompat break for Facebook. + $this->setTasks($value); + continue; + } + + if (empty($aux_fields[$field])) { + throw new Exception( + "Parsed commit message contains unrecognized field '{$field}'."); + } + $aux_fields[$field]->setValueFromParsedCommitMessage($value); + } + + $aux_fields = array_values($aux_fields); + $this->setAuxiliaryFields($aux_fields); } public function setAuxiliaryFields(array $auxiliary_fields) { diff --git a/src/applications/differential/editor/revision/__init__.php b/src/applications/differential/editor/revision/__init__.php index 1b6a0cc33b..231afd8eff 100644 --- a/src/applications/differential/editor/revision/__init__.php +++ b/src/applications/differential/editor/revision/__init__.php @@ -8,6 +8,7 @@ phutil_require_module('phabricator', 'applications/differential/constants/action'); phutil_require_module('phabricator', 'applications/differential/constants/revisionstatus'); +phutil_require_module('phabricator', 'applications/differential/field/selector/base'); phutil_require_module('phabricator', 'applications/differential/mail/ccwelcome'); phutil_require_module('phabricator', 'applications/differential/mail/newdiff'); phutil_require_module('phabricator', 'applications/differential/storage/auxiliaryfield'); diff --git a/src/applications/differential/field/selector/default/DifferentialDefaultFieldSelector.php b/src/applications/differential/field/selector/default/DifferentialDefaultFieldSelector.php index 6249b62d30..c8dd382fe0 100644 --- a/src/applications/differential/field/selector/default/DifferentialDefaultFieldSelector.php +++ b/src/applications/differential/field/selector/default/DifferentialDefaultFieldSelector.php @@ -27,6 +27,7 @@ final class DifferentialDefaultFieldSelector new DifferentialRevisionStatusFieldSpecification(), new DifferentialAuthorFieldSpecification(), new DifferentialReviewersFieldSpecification(), + new DifferentialReviewedByFieldSpecification(), new DifferentialCCsFieldSpecification(), new DifferentialUnitFieldSpecification(), new DifferentialLintFieldSpecification(), @@ -39,6 +40,15 @@ final class DifferentialDefaultFieldSelector new DifferentialArcanistProjectFieldSpecification(), new DifferentialApplyPatchFieldSpecification(), new DifferentialExportPatchFieldSpecification(), + new DifferentialRevisionIDFieldSpecification(), + new DifferentialGitSVNIDFieldSpecification(), + + // TODO: Remove these from the default set, we need them temporarily + // because the commit message dictionary always contains their keys + // right now. + new DifferentialBlameRevisionFieldSpecification(), + new DifferentialRevertPlanFieldSpecification(), + ); } diff --git a/src/applications/differential/field/selector/default/__init__.php b/src/applications/differential/field/selector/default/__init__.php index 7c02e0fc83..80ad649b78 100644 --- a/src/applications/differential/field/selector/default/__init__.php +++ b/src/applications/differential/field/selector/default/__init__.php @@ -10,16 +10,21 @@ phutil_require_module('phabricator', 'applications/differential/field/selector/b phutil_require_module('phabricator', 'applications/differential/field/specification/applypatch'); phutil_require_module('phabricator', 'applications/differential/field/specification/arcanistproject'); phutil_require_module('phabricator', 'applications/differential/field/specification/author'); +phutil_require_module('phabricator', 'applications/differential/field/specification/blamerev'); phutil_require_module('phabricator', 'applications/differential/field/specification/ccs'); phutil_require_module('phabricator', 'applications/differential/field/specification/commits'); phutil_require_module('phabricator', 'applications/differential/field/specification/dependencies'); phutil_require_module('phabricator', 'applications/differential/field/specification/exportpatch'); +phutil_require_module('phabricator', 'applications/differential/field/specification/gitsvnid'); phutil_require_module('phabricator', 'applications/differential/field/specification/host'); phutil_require_module('phabricator', 'applications/differential/field/specification/lines'); phutil_require_module('phabricator', 'applications/differential/field/specification/lint'); phutil_require_module('phabricator', 'applications/differential/field/specification/maniphesttasks'); phutil_require_module('phabricator', 'applications/differential/field/specification/path'); +phutil_require_module('phabricator', 'applications/differential/field/specification/revertplan'); +phutil_require_module('phabricator', 'applications/differential/field/specification/reviewedby'); phutil_require_module('phabricator', 'applications/differential/field/specification/reviewers'); +phutil_require_module('phabricator', 'applications/differential/field/specification/revisionid'); phutil_require_module('phabricator', 'applications/differential/field/specification/revisionstatus'); phutil_require_module('phabricator', 'applications/differential/field/specification/summary'); phutil_require_module('phabricator', 'applications/differential/field/specification/testplan'); diff --git a/src/applications/differential/field/specification/base/DifferentialFieldSpecification.php b/src/applications/differential/field/specification/base/DifferentialFieldSpecification.php index 0c2d98dadc..96b5103000 100644 --- a/src/applications/differential/field/specification/base/DifferentialFieldSpecification.php +++ b/src/applications/differential/field/specification/base/DifferentialFieldSpecification.php @@ -28,6 +28,7 @@ * @task edit Extending the Revision Edit Interface * @task view Extending the Revision View Interface * @task conduit Extending the Conduit View Interface + * @task commit Extending Commit Messages * @task load Loading Additional Data * @task context Contextual Data */ @@ -177,6 +178,16 @@ abstract class DifferentialFieldSpecification { } /** + * Hook for applying revision changes via the editor. Normally, you should + * not implement this, but a number of builtin fields use the revision object + * itself as storage. If you need to do something similar for whatever reason, + * this method gives you an opportunity to interact with the editor or + * revision before changes are saved (for example, you can write the field's + * value into some property of the revision). + * + * @param DifferentialRevisionEditor Active editor which is applying changes + * to the revision. + * @return void * @task edit */ public function willWriteRevision(DifferentialRevisionEditor $editor) { @@ -184,6 +195,14 @@ abstract class DifferentialFieldSpecification { } /** + * Hook after an edit operation has completed. This allows you to update + * link tables or do other write operations which should happen after the + * revision is saved. Normally you don't need to implement this. + * + * + * @param DifferentialRevisionEditor Active editor which has just applied + * changes to the revision. + * @return void * @task edit */ public function didWriteRevision(DifferentialRevisionEditor $editor) { @@ -273,6 +292,63 @@ abstract class DifferentialFieldSpecification { } +/* -( Extending Commit Messages )------------------------------------------ */ + + + /** + * Determine if this field should appear in commit messages. You should return + * true if this field participates in any part of the commit message workflow, + * even if it is not rendered by default. + * + * If you implement this method, you must implement + * @{method:getCommitMessageKey} and + * @{method:setValueFromParsedCommitMessage}. + * + * @return bool True if this field appears in commit messages in any capacity. + * @task commit + */ + public function shouldAppearOnCommitMessage() { + return false; + } + + /** + * Key which identifies this field in parsed commit messages. Commit messages + * exist in two forms: raw textual commit messages and parsed dictionaries of + * fields. This method must return a unique string which identifies this field + * in dictionaries. Principally, this dictionary is shipped to and from arc + * over Conduit. Keys should be appropriate property names, like "testPlan" + * (not "Test Plan") and must be globally unique. + * + * You must implement this method if you return true from + * @{method:shouldAppearOnCommitMessage}. + * + * @return string Key which identifies the field in dictionaries. + * @task commit + */ + public function getCommitMessageKey() { + throw new DifferentialFieldSpecificationIncompleteException($this); + } + + /** + * Set this field's value from a value in a parsed commit message dictionary. + * Afterward, this field will go through the normal write workflows and the + * change will be permanently stored via either the storage mechanisms (if + * your field implements them), revision write hooks (if your field implements + * them) or discarded (if your field implements neither, e.g. is just a + * display field). + * + * You must implement this method if you return true from + * @{method:shouldAppearOnCommitMessage}. + * + * @param mixed Field value from a parsed commit message dictionary. + * @return this + * @task commit + */ + public function setValueFromParsedCommitMessage($value) { + throw new DifferentialFieldSpecificationIncompleteException($this); + } + + /* -( Loading Additional Data )-------------------------------------------- */ diff --git a/src/applications/differential/field/specification/blamerev/DifferentialBlameRevisionFieldSpecification.php b/src/applications/differential/field/specification/blamerev/DifferentialBlameRevisionFieldSpecification.php index 6c877c2024..c058590cee 100644 --- a/src/applications/differential/field/specification/blamerev/DifferentialBlameRevisionFieldSpecification.php +++ b/src/applications/differential/field/specification/blamerev/DifferentialBlameRevisionFieldSpecification.php @@ -78,4 +78,17 @@ final class DifferentialBlameRevisionFieldSpecification return $this->value; } + public function shouldAppearOnCommitMessage() { + return true; + } + + public function getCommitMessageKey() { + return 'blameRevision'; + } + + public function setValueFromParsedCommitMessage($value) { + $this->value = $value; + return $this; + } + } diff --git a/src/applications/differential/field/specification/ccs/DifferentialCCsFieldSpecification.php b/src/applications/differential/field/specification/ccs/DifferentialCCsFieldSpecification.php index 672dcab5fd..6bf514fb7f 100644 --- a/src/applications/differential/field/specification/ccs/DifferentialCCsFieldSpecification.php +++ b/src/applications/differential/field/specification/ccs/DifferentialCCsFieldSpecification.php @@ -82,4 +82,17 @@ final class DifferentialCCsFieldSpecification $editor->setCCPHIDs($this->ccs); } + public function shouldAppearOnCommitMessage() { + return true; + } + + public function getCommitMessageKey() { + return 'ccPHIDs'; + } + + public function setValueFromParsedCommitMessage($value) { + $this->value = nonempty($value, array()); + return $this; + } + } diff --git a/src/applications/differential/field/specification/gitsvnid/DifferentialGitSVNIDFieldSpecification.php b/src/applications/differential/field/specification/gitsvnid/DifferentialGitSVNIDFieldSpecification.php new file mode 100644 index 0000000000..c908946303 --- /dev/null +++ b/src/applications/differential/field/specification/gitsvnid/DifferentialGitSVNIDFieldSpecification.php @@ -0,0 +1,37 @@ +gitSVNID = $value; + return $this; + } + +} diff --git a/src/applications/differential/field/specification/gitsvnid/__init__.php b/src/applications/differential/field/specification/gitsvnid/__init__.php new file mode 100644 index 0000000000..b380586817 --- /dev/null +++ b/src/applications/differential/field/specification/gitsvnid/__init__.php @@ -0,0 +1,12 @@ +value; } + public function shouldAppearOnCommitMessage() { + return true; + } + + public function getCommitMessageKey() { + return 'revertPlan'; + } + + public function setValueFromParsedCommitMessage($value) { + $this->value = $value; + return $this; + } + } diff --git a/src/applications/differential/field/specification/reviewedby/DifferentialReviewedByFieldSpecification.php b/src/applications/differential/field/specification/reviewedby/DifferentialReviewedByFieldSpecification.php new file mode 100644 index 0000000000..f5904e2ee2 --- /dev/null +++ b/src/applications/differential/field/specification/reviewedby/DifferentialReviewedByFieldSpecification.php @@ -0,0 +1,38 @@ +reviewedBy = $value; + return $this; + } + + +} diff --git a/src/applications/differential/field/specification/reviewedby/__init__.php b/src/applications/differential/field/specification/reviewedby/__init__.php new file mode 100644 index 0000000000..a87a459dbd --- /dev/null +++ b/src/applications/differential/field/specification/reviewedby/__init__.php @@ -0,0 +1,12 @@ +setReviewers($this->reviewers); } + public function shouldAppearOnCommitMessage() { + return true; + } + + public function getCommitMessageKey() { + return 'reviewerPHIDs'; + } + + public function setValueFromParsedCommitMessage($value) { + $this->reviewers = nonempty($value, array()); + return $this; + } } diff --git a/src/applications/differential/field/specification/revisionid/DifferentialRevisionIDFieldSpecification.php b/src/applications/differential/field/specification/revisionid/DifferentialRevisionIDFieldSpecification.php new file mode 100644 index 0000000000..e1f1cf30b3 --- /dev/null +++ b/src/applications/differential/field/specification/revisionid/DifferentialRevisionIDFieldSpecification.php @@ -0,0 +1,38 @@ +id = $value; + return $this; + } + + +} diff --git a/src/applications/differential/field/specification/revisionid/__init__.php b/src/applications/differential/field/specification/revisionid/__init__.php new file mode 100644 index 0000000000..1b3dc16ecc --- /dev/null +++ b/src/applications/differential/field/specification/revisionid/__init__.php @@ -0,0 +1,12 @@ +getRevision()->setSummary($this->summary); } + public function shouldAppearOnCommitMessage() { + return true; + } + + public function getCommitMessageKey() { + return 'summary'; + } + + public function setValueFromParsedCommitMessage($value) { + $this->summary = $value; + return $this; + } + } diff --git a/src/applications/differential/field/specification/testplan/DifferentialTestPlanFieldSpecification.php b/src/applications/differential/field/specification/testplan/DifferentialTestPlanFieldSpecification.php index 6d4d3bcb6d..8eed5dc248 100644 --- a/src/applications/differential/field/specification/testplan/DifferentialTestPlanFieldSpecification.php +++ b/src/applications/differential/field/specification/testplan/DifferentialTestPlanFieldSpecification.php @@ -53,4 +53,18 @@ final class DifferentialTestPlanFieldSpecification } } + public function shouldAppearOnCommitMessage() { + return true; + } + + public function getCommitMessageKey() { + return 'testPlan'; + } + + public function setValueFromParsedCommitMessage($value) { + $this->plan = $value; + return $this; + } + + } diff --git a/src/applications/differential/field/specification/title/DifferentialTitleFieldSpecification.php b/src/applications/differential/field/specification/title/DifferentialTitleFieldSpecification.php index e62bc0626d..7b6bb94dc2 100644 --- a/src/applications/differential/field/specification/title/DifferentialTitleFieldSpecification.php +++ b/src/applications/differential/field/specification/title/DifferentialTitleFieldSpecification.php @@ -54,4 +54,17 @@ final class DifferentialTitleFieldSpecification } } + public function shouldAppearOnCommitMessage() { + return true; + } + + public function getCommitMessageKey() { + return 'title'; + } + + public function setValueFromParsedCommitMessage($value) { + $this->title = $value; + return $this; + } + }