Implement all field edit interfaces on the custom field schema

Summary:
Moves the revision edit controller to be completely schema-driven.

Depends on D810.

Test Plan: Edited revisions. Entered intentionally invalid values to trigger
error conditions.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, jungejason, epriestley
Differential Revision: 810

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',

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');
+    foreach ($aux_fields as $key => $aux_field) {
-
+      $aux_field->setHandles(array_select_keys($handles, $aux_phids[$key]));
-    $reviewer_map = array_select_keys($handles, $reviewer_phids);
+    }
-    $cc_map = array_select_keys($handles, $cc_phids);
 
     $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);
       }
     }
 

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');
 

src/applications/differential/editor/revision/DifferentialRevisionEditor.php

@@ -672,10 +672,11 @@ class DifferentialRevisionEditor {
     $aux_map = array();
     foreach ($this->auxiliaryFields as $aux_field) {
       $key = $aux_field->getStorageKey();
+      if ($key !== null) {
         $val = $aux_field->getValueForStorage();
-
         $aux_map[$key] = $val;
       }
+    }
 
     if (!$aux_map) {
       return;

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(),

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');
 
 

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)

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);
+  }
+
 }

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');

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);
+  }
+
+
 }

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');

src/applications/differential/field/specification/summary/DifferentialSummaryFieldSpecification.php

@@ -0,0 +1,45 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+final class DifferentialSummaryFieldSpecification
+  extends DifferentialFieldSpecification {
+
+  private $summary;
+
+  public function shouldAppearOnEdit() {
+    $this->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);
+  }
+
+}

src/applications/differential/field/specification/summary/__init__.php

@@ -0,0 +1,15 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'applications/differential/field/specification/base');
+phutil_require_module('phabricator', 'view/form/control/textarea');
+
+phutil_require_module('phutil', 'utils');
+
+
+phutil_require_source('DifferentialSummaryFieldSpecification.php');

src/applications/differential/field/specification/testplan/DifferentialTestPlanFieldSpecification.php

@@ -0,0 +1,56 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+final class DifferentialTestPlanFieldSpecification
+  extends DifferentialFieldSpecification {
+
+  private $plan;
+  private $error = true;
+
+  public function shouldAppearOnEdit() {
+    $this->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.");
+    }
+  }
+
+}

src/applications/differential/field/specification/testplan/__init__.php

@@ -0,0 +1,16 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+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/textarea');
+
+phutil_require_module('phutil', 'utils');
+
+
+phutil_require_source('DifferentialTestPlanFieldSpecification.php');

src/applications/differential/field/specification/title/DifferentialTitleFieldSpecification.php

@@ -0,0 +1,57 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+final class DifferentialTitleFieldSpecification
+  extends DifferentialFieldSpecification {
+
+  private $title;
+  private $error = true;
+
+  public function shouldAppearOnEdit() {
+    $this->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.");
+    }
+  }
+
+}

src/applications/differential/field/specification/title/__init__.php

@@ -0,0 +1,16 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+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/textarea');
+
+phutil_require_module('phutil', 'utils');
+
+
+phutil_require_source('DifferentialTitleFieldSpecification.php');