diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d40b8c6805..2bfa0b596a 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -207,7 +207,10 @@ phutil_register_library_map(array( 'DifferentialRevisionUpdateHistoryView' => 'applications/differential/view/revisionupdatehistory', 'DifferentialRevisionViewController' => 'applications/differential/controller/revisionview', 'DifferentialSubscribeController' => 'applications/differential/controller/subscribe', + 'DifferentialSummaryFieldSpecification' => 'applications/differential/field/specification/summary', 'DifferentialTasksAttacher' => 'applications/differential/tasks', + 'DifferentialTestPlanFieldSpecification' => 'applications/differential/field/specification/testplan', + 'DifferentialTitleFieldSpecification' => 'applications/differential/field/specification/title', 'DifferentialUnitFieldSpecification' => 'applications/differential/field/specification/unit', 'DifferentialUnitStatus' => 'applications/differential/constants/unitstatus', 'DifferentialUnitTestResult' => 'applications/differential/constants/unittestresult', @@ -845,6 +848,9 @@ phutil_register_library_map(array( 'DifferentialRevisionUpdateHistoryView' => 'AphrontView', 'DifferentialRevisionViewController' => 'DifferentialController', 'DifferentialSubscribeController' => 'DifferentialController', + 'DifferentialSummaryFieldSpecification' => 'DifferentialFieldSpecification', + 'DifferentialTestPlanFieldSpecification' => 'DifferentialFieldSpecification', + 'DifferentialTitleFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialUnitFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialViewTime' => 'DifferentialDAO', 'DiffusionBranchTableView' => 'DiffusionView', diff --git a/src/applications/differential/controller/revisionedit/DifferentialRevisionEditController.php b/src/applications/differential/controller/revisionedit/DifferentialRevisionEditController.php index 421585e318..a0c6c6662c 100644 --- a/src/applications/differential/controller/revisionedit/DifferentialRevisionEditController.php +++ b/src/applications/differential/controller/revisionedit/DifferentialRevisionEditController.php @@ -41,6 +41,7 @@ class DifferentialRevisionEditController extends DifferentialController { $revision = new DifferentialRevision(); } + $revision->loadRelationships(); $aux_fields = $this->loadAuxiliaryFields($revision); $diff_id = $request->getInt('diffID'); @@ -57,39 +58,12 @@ class DifferentialRevisionEditController extends DifferentialController { $diff = null; } - $e_title = true; - $e_testplan = true; - $e_reviewers = null; $errors = array(); - $revision->loadRelationships(); if ($request->isFormPost() && !$request->getStr('viaDiffView')) { - $revision->setTitle($request->getStr('title')); - $revision->setSummary($request->getStr('summary')); - $revision->setTestPlan($request->getStr('testplan')); - - if (!strlen(trim($revision->getTitle()))) { - $errors[] = 'You must provide a title.'; - $e_title = 'Required'; - } else { - $e_title = null; - } - - if (!strlen(trim($revision->getTestPlan()))) { - $errors[] = 'You must provide a test plan.'; - $e_testplan = 'Required'; - } else { - $e_testplan = null; - } - $user_phid = $request->getUser()->getPHID(); - if (in_array($user_phid, $request->getArr('reviewers'))) { - $errors[] = 'You may not review your own revision.'; - $e_reviewers = 'Invalid'; - } - foreach ($aux_fields as $aux_field) { $aux_field->setValueFromRequest($request); try { @@ -105,30 +79,24 @@ class DifferentialRevisionEditController extends DifferentialController { $editor->addDiff($diff, $request->getStr('comments')); } $editor->setAuxiliaryFields($aux_fields); - $editor->setCCPHIDs($request->getArr('cc')); - $editor->setReviewers($request->getArr('reviewers')); $editor->save(); return id(new AphrontRedirectResponse()) ->setURI('/D'.$revision->getID()); } - - $reviewer_phids = $request->getArr('reviewers'); - $cc_phids = $request->getArr('cc'); - } else { - $reviewer_phids = $revision->getReviewers(); - $cc_phids = $revision->getCCPHIDs(); } - $phids = array_merge($reviewer_phids, $cc_phids); + $aux_phids = array(); + foreach ($aux_fields as $key => $aux_field) { + $aux_phids[$key] = $aux_field->getRequiredHandlePHIDsForRevisionEdit(); + } + $phids = array_mergev($aux_phids); $phids = array_unique($phids); - $handles = id(new PhabricatorObjectHandleData($phids)) ->loadHandles(); - $handles = mpull($handles, 'getFullName', 'getPHID'); - - $reviewer_map = array_select_keys($handles, $reviewer_phids); - $cc_map = array_select_keys($handles, $cc_phids); + foreach ($aux_fields as $key => $aux_field) { + $aux_field->setHandles(array_select_keys($handles, $aux_phids[$key])); + } $form = new AphrontFormView(); $form->setUser($request->getUser()); @@ -164,39 +132,6 @@ class DifferentialRevisionEditController extends DifferentialController { id(new AphrontFormDividerControl())); } - $form - ->appendChild( - id(new AphrontFormTextAreaControl()) - ->setLabel('Title') - ->setName('title') - ->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_SHORT) - ->setValue($revision->getTitle()) - ->setError($e_title)) - ->appendChild( - id(new AphrontFormTextAreaControl()) - ->setLabel('Summary') - ->setName('summary') - ->setValue($revision->getSummary())) - ->appendChild( - id(new AphrontFormTextAreaControl()) - ->setLabel('Test Plan') - ->setName('testplan') - ->setValue($revision->getTestPlan()) - ->setError($e_testplan)) - ->appendChild( - id(new AphrontFormTokenizerControl()) - ->setLabel('Reviewers') - ->setName('reviewers') - ->setDatasource('/typeahead/common/users/') - ->setError($e_reviewers) - ->setValue($reviewer_map)) - ->appendChild( - id(new AphrontFormTokenizerControl()) - ->setLabel('CC') - ->setName('cc') - ->setDatasource('/typeahead/common/mailable/') - ->setValue($cc_map)); - foreach ($aux_fields as $aux_field) { $control = $aux_field->renderEditControl(); if ($control) { @@ -237,11 +172,16 @@ class DifferentialRevisionEditController extends DifferentialController { private function loadAuxiliaryFields(DifferentialRevision $revision) { + $user = $this->getRequest()->getUser(); + $aux_fields = DifferentialFieldSelector::newSelector() ->getFieldSpecifications(); foreach ($aux_fields as $key => $aux_field) { + $aux_field->setRevision($revision); if (!$aux_field->shouldAppearOnEdit()) { unset($aux_fields[$key]); + } else { + $aux_field->setUser($user); } } diff --git a/src/applications/differential/controller/revisionedit/__init__.php b/src/applications/differential/controller/revisionedit/__init__.php index c29c38c212..ca7bd99182 100644 --- a/src/applications/differential/controller/revisionedit/__init__.php +++ b/src/applications/differential/controller/revisionedit/__init__.php @@ -19,7 +19,6 @@ phutil_require_module('phabricator', 'view/form/base'); phutil_require_module('phabricator', 'view/form/control/divider'); phutil_require_module('phabricator', 'view/form/control/submit'); phutil_require_module('phabricator', 'view/form/control/textarea'); -phutil_require_module('phabricator', 'view/form/control/tokenizer'); phutil_require_module('phabricator', 'view/form/error'); phutil_require_module('phabricator', 'view/layout/panel'); diff --git a/src/applications/differential/editor/revision/DifferentialRevisionEditor.php b/src/applications/differential/editor/revision/DifferentialRevisionEditor.php index 6611492e51..b5bb7075b4 100644 --- a/src/applications/differential/editor/revision/DifferentialRevisionEditor.php +++ b/src/applications/differential/editor/revision/DifferentialRevisionEditor.php @@ -672,9 +672,10 @@ class DifferentialRevisionEditor { $aux_map = array(); foreach ($this->auxiliaryFields as $aux_field) { $key = $aux_field->getStorageKey(); - $val = $aux_field->getValueForStorage(); - - $aux_map[$key] = $val; + if ($key !== null) { + $val = $aux_field->getValueForStorage(); + $aux_map[$key] = $val; + } } if (!$aux_map) { diff --git a/src/applications/differential/field/selector/default/DifferentialDefaultFieldSelector.php b/src/applications/differential/field/selector/default/DifferentialDefaultFieldSelector.php index 0c9380c8ac..6249b62d30 100644 --- a/src/applications/differential/field/selector/default/DifferentialDefaultFieldSelector.php +++ b/src/applications/differential/field/selector/default/DifferentialDefaultFieldSelector.php @@ -21,6 +21,9 @@ final class DifferentialDefaultFieldSelector public function getFieldSpecifications() { return array( + new DifferentialTitleFieldSpecification(), + new DifferentialSummaryFieldSpecification(), + new DifferentialTestPlanFieldSpecification(), new DifferentialRevisionStatusFieldSpecification(), new DifferentialAuthorFieldSpecification(), new DifferentialReviewersFieldSpecification(), diff --git a/src/applications/differential/field/selector/default/__init__.php b/src/applications/differential/field/selector/default/__init__.php index 85864635b0..7c02e0fc83 100644 --- a/src/applications/differential/field/selector/default/__init__.php +++ b/src/applications/differential/field/selector/default/__init__.php @@ -21,6 +21,9 @@ phutil_require_module('phabricator', 'applications/differential/field/specificat phutil_require_module('phabricator', 'applications/differential/field/specification/path'); phutil_require_module('phabricator', 'applications/differential/field/specification/reviewers'); 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'); +phutil_require_module('phabricator', 'applications/differential/field/specification/title'); phutil_require_module('phabricator', 'applications/differential/field/specification/unit'); diff --git a/src/applications/differential/field/specification/base/DifferentialFieldSpecification.php b/src/applications/differential/field/specification/base/DifferentialFieldSpecification.php index ae036cfeb4..0c2d98dadc 100644 --- a/src/applications/differential/field/specification/base/DifferentialFieldSpecification.php +++ b/src/applications/differential/field/specification/base/DifferentialFieldSpecification.php @@ -37,6 +37,7 @@ abstract class DifferentialFieldSpecification { private $diff; private $handles; private $diffProperties; + private $user; /* -( Storage )------------------------------------------------------------ */ @@ -321,7 +322,7 @@ abstract class DifferentialFieldSpecification { * @return list List of PHIDs to load handles for. * @task load */ - public function getRequiredHandlePHIDsForEdit() { + public function getRequiredHandlePHIDsForRevisionEdit() { return $this->getRequiredHandlePHIDs(); } @@ -372,6 +373,14 @@ abstract class DifferentialFieldSpecification { return $this; } + /** + * @task context + */ + final public function setUser(PhabricatorUser $user) { + $this->user = $user; + return $this; + } + /** * @task context */ @@ -392,6 +401,16 @@ abstract class DifferentialFieldSpecification { return $this->diff; } + /** + * @task context + */ + final protected function getUser() { + if (empty($this->user)) { + throw new DifferentialFieldDataNotAvailableException($this); + } + return $this->user; + } + /** * Get the handle for an object PHID. You must overload * @{method:getRequiredHandlePHIDs} (or a more specific version thereof) diff --git a/src/applications/differential/field/specification/ccs/DifferentialCCsFieldSpecification.php b/src/applications/differential/field/specification/ccs/DifferentialCCsFieldSpecification.php index ed57ab83fc..672dcab5fd 100644 --- a/src/applications/differential/field/specification/ccs/DifferentialCCsFieldSpecification.php +++ b/src/applications/differential/field/specification/ccs/DifferentialCCsFieldSpecification.php @@ -19,6 +19,8 @@ final class DifferentialCCsFieldSpecification extends DifferentialFieldSpecification { + private $ccs; + public function shouldAppearOnRevisionView() { return true; } @@ -50,4 +52,34 @@ final class DifferentialCCsFieldSpecification return $revision->getCCPHIDs(); } + public function shouldAppearOnEdit() { + $this->ccs = $this->getCCPHIDs(); + return true; + } + + public function getRequiredHandlePHIDsForRevisionEdit() { + return $this->ccs; + } + + public function setValueFromRequest(AphrontRequest $request) { + $this->ccs = $request->getArr('cc'); + return $this; + } + + public function renderEditControl() { + $cc_map = array(); + foreach ($this->ccs as $phid) { + $cc_map[$phid] = $this->getHandle($phid)->getFullName(); + } + return id(new AphrontFormTokenizerControl()) + ->setLabel('CC') + ->setName('cc') + ->setDatasource('/typeahead/common/mailable/') + ->setValue($cc_map); + } + + public function willWriteRevision(DifferentialRevisionEditor $editor) { + $editor->setCCPHIDs($this->ccs); + } + } diff --git a/src/applications/differential/field/specification/ccs/__init__.php b/src/applications/differential/field/specification/ccs/__init__.php index f7e1685565..0713238f4d 100644 --- a/src/applications/differential/field/specification/ccs/__init__.php +++ b/src/applications/differential/field/specification/ccs/__init__.php @@ -7,6 +7,9 @@ phutil_require_module('phabricator', 'applications/differential/field/specification/base'); +phutil_require_module('phabricator', 'view/form/control/tokenizer'); + +phutil_require_module('phutil', 'utils'); phutil_require_source('DifferentialCCsFieldSpecification.php'); diff --git a/src/applications/differential/field/specification/reviewers/DifferentialReviewersFieldSpecification.php b/src/applications/differential/field/specification/reviewers/DifferentialReviewersFieldSpecification.php index 909087db72..7824fd0219 100644 --- a/src/applications/differential/field/specification/reviewers/DifferentialReviewersFieldSpecification.php +++ b/src/applications/differential/field/specification/reviewers/DifferentialReviewersFieldSpecification.php @@ -19,6 +19,9 @@ final class DifferentialReviewersFieldSpecification extends DifferentialFieldSpecification { + private $reviewers; + private $error; + public function shouldAppearOnRevisionView() { return true; } @@ -50,4 +53,44 @@ final class DifferentialReviewersFieldSpecification return $revision->getReviewers(); } + public function shouldAppearOnEdit() { + $this->reviewers = $this->getReviewerPHIDs(); + return true; + } + + public function getRequiredHandlePHIDsForRevisionEdit() { + return $this->reviewers; + } + + public function setValueFromRequest(AphrontRequest $request) { + $this->reviewers = $request->getArr('reviewers'); + return $this; + } + + public function validateField() { + if (in_array($this->getUser()->getPHID(), $this->reviewers)) { + $this->error = 'Invalid'; + throw new DifferentialFieldValidationException( + "You may not review your own revision!"); + } + } + + public function renderEditControl() { + $reviewer_map = array(); + foreach ($this->reviewers as $phid) { + $reviewer_map[$phid] = $this->getHandle($phid)->getFullName(); + } + return id(new AphrontFormTokenizerControl()) + ->setLabel('Reviewers') + ->setName('reviewers') + ->setDatasource('/typeahead/common/users/') + ->setValue($reviewer_map) + ->setError($this->error); + } + + public function willWriteRevision(DifferentialRevisionEditor $editor) { + $editor->setReviewers($this->reviewers); + } + + } diff --git a/src/applications/differential/field/specification/reviewers/__init__.php b/src/applications/differential/field/specification/reviewers/__init__.php index fb1adf0a61..fbd31d45cd 100644 --- a/src/applications/differential/field/specification/reviewers/__init__.php +++ b/src/applications/differential/field/specification/reviewers/__init__.php @@ -6,7 +6,11 @@ +phutil_require_module('phabricator', 'applications/differential/field/exception/validation'); phutil_require_module('phabricator', 'applications/differential/field/specification/base'); +phutil_require_module('phabricator', 'view/form/control/tokenizer'); + +phutil_require_module('phutil', 'utils'); phutil_require_source('DifferentialReviewersFieldSpecification.php'); diff --git a/src/applications/differential/field/specification/summary/DifferentialSummaryFieldSpecification.php b/src/applications/differential/field/specification/summary/DifferentialSummaryFieldSpecification.php new file mode 100644 index 0000000000..177112fb94 --- /dev/null +++ b/src/applications/differential/field/specification/summary/DifferentialSummaryFieldSpecification.php @@ -0,0 +1,45 @@ +summary = $this->getRevision()->getSummary(); + return true; + } + + public function setValueFromRequest(AphrontRequest $request) { + $this->summary = $request->getStr('summary'); + return $this; + } + + public function renderEditControl() { + return id(new AphrontFormTextAreaControl()) + ->setLabel('Summary') + ->setName('summary') + ->setValue($this->summary); + } + + public function willWriteRevision(DifferentialRevisionEditor $editor) { + $this->getRevision()->setSummary($this->summary); + } + +} diff --git a/src/applications/differential/field/specification/summary/__init__.php b/src/applications/differential/field/specification/summary/__init__.php new file mode 100644 index 0000000000..e7204ed0e1 --- /dev/null +++ b/src/applications/differential/field/specification/summary/__init__.php @@ -0,0 +1,15 @@ +plan = $this->getRevision()->getTestPlan(); + return true; + } + + public function setValueFromRequest(AphrontRequest $request) { + $this->plan = $request->getStr('testplan'); + $this->error = null; + return $this; + } + + public function renderEditControl() { + return id(new AphrontFormTextAreaControl()) + ->setLabel('Test Plan') + ->setName('testplan') + ->setValue($this->plan) + ->setError($this->error); + } + + public function willWriteRevision(DifferentialRevisionEditor $editor) { + $this->getRevision()->setTestPlan($this->plan); + } + + public function validateField() { + if (!strlen($this->plan)) { + $this->error = 'Required'; + throw new DifferentialFieldValidationException( + "You must provide a test plan."); + } + } + +} diff --git a/src/applications/differential/field/specification/testplan/__init__.php b/src/applications/differential/field/specification/testplan/__init__.php new file mode 100644 index 0000000000..73db961b93 --- /dev/null +++ b/src/applications/differential/field/specification/testplan/__init__.php @@ -0,0 +1,16 @@ +title = $this->getRevision()->getTitle(); + return true; + } + + public function setValueFromRequest(AphrontRequest $request) { + $this->title = $request->getStr('title'); + $this->error = null; + return $this; + } + + public function renderEditControl() { + return id(new AphrontFormTextAreaControl()) + ->setLabel('Title') + ->setName('title') + ->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_SHORT) + ->setError($this->error) + ->setValue($this->title); + } + + public function willWriteRevision(DifferentialRevisionEditor $editor) { + $this->getRevision()->setTitle($this->title); + } + + public function validateField() { + if (!strlen($this->title)) { + $this->error = 'Required'; + throw new DifferentialFieldValidationException( + "You must provide a title."); + } + } + +} diff --git a/src/applications/differential/field/specification/title/__init__.php b/src/applications/differential/field/specification/title/__init__.php new file mode 100644 index 0000000000..ddc512831d --- /dev/null +++ b/src/applications/differential/field/specification/title/__init__.php @@ -0,0 +1,16 @@ +