Support enabling a formal points field in Maniphest

Summary:
Ref T4427.

  - New config option for labels, enabling, etc., but no UI/niceness yet.
  - When enabled, add a field.
  - Allow nonnegative values, including fractional values.
  - EditEngine is nice and Conduit / actions basically just work with a tiny bit of extra support code.

Test Plan:
  - Edited points via "Edit".
  - Edited points via Conduit.
  - Edited points via stacked actions.
  - Tried to set "zebra" points.
  - Tried to set -1 points.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4427

Differential Revision: https://secure.phabricator.com/D15220

resources/celerity/map.php

@@ -507,7 +507,7 @@ return array(
     'rsrc/js/phuix/PHUIXActionView.js' => '8cf6d262',
     'rsrc/js/phuix/PHUIXAutocomplete.js' => '9196fb06',
     'rsrc/js/phuix/PHUIXDropdownMenu.js' => 'bd4c8dca',
-    'rsrc/js/phuix/PHUIXFormControl.js' => '8fba1997',
+    'rsrc/js/phuix/PHUIXFormControl.js' => 'a7763e11',
     'rsrc/js/phuix/PHUIXIconView.js' => 'bff6884b',
   ),
   'symbols' => array(
@@ -840,7 +840,7 @@ return array(
     'phuix-action-view' => '8cf6d262',
     'phuix-autocomplete' => '9196fb06',
     'phuix-dropdown-menu' => 'bd4c8dca',
-    'phuix-form-control-view' => '8fba1997',
+    'phuix-form-control-view' => 'a7763e11',
     'phuix-icon-view' => 'bff6884b',
     'policy-css' => '957ea14c',
     'policy-edit-css' => '815c66f7',
@@ -1526,10 +1526,6 @@ return array(
       'javelin-stratcom',
       'javelin-install',
     ),
-    '8fba1997' => array(
-      'javelin-install',
-      'javelin-dom',
-    ),
     '901935ef' => array(
       'javelin-behavior',
       'javelin-dom',
@@ -1633,6 +1629,10 @@ return array(
       'javelin-uri',
       'phabricator-notification',
     ),
+    'a7763e11' => array(
+      'javelin-install',
+      'javelin-dom',
+    ),
     'a78c0661' => array(
       'phui-workcard-view-css',
     ),

src/__phutil_library_map__.php

@@ -241,6 +241,7 @@ phutil_register_library_map(array(
     'ConduitPHIDParameterType' => 'applications/conduit/parametertype/ConduitPHIDParameterType.php',
     'ConduitParameterType' => 'applications/conduit/parametertype/ConduitParameterType.php',
     'ConduitPingConduitAPIMethod' => 'applications/conduit/method/ConduitPingConduitAPIMethod.php',
+    'ConduitPointsParameterType' => 'applications/conduit/parametertype/ConduitPointsParameterType.php',
     'ConduitProjectListParameterType' => 'applications/conduit/parametertype/ConduitProjectListParameterType.php',
     'ConduitQueryConduitAPIMethod' => 'applications/conduit/method/ConduitQueryConduitAPIMethod.php',
     'ConduitResultSearchEngineExtension' => 'applications/conduit/query/ConduitResultSearchEngineExtension.php',
@@ -1339,6 +1340,7 @@ phutil_register_library_map(array(
     'ManiphestTaskOpenStatusDatasource' => 'applications/maniphest/typeahead/ManiphestTaskOpenStatusDatasource.php',
     'ManiphestTaskPHIDResolver' => 'applications/maniphest/httpparametertype/ManiphestTaskPHIDResolver.php',
     'ManiphestTaskPHIDType' => 'applications/maniphest/phid/ManiphestTaskPHIDType.php',
+    'ManiphestTaskPoints' => 'applications/maniphest/constants/ManiphestTaskPoints.php',
     'ManiphestTaskPriority' => 'applications/maniphest/constants/ManiphestTaskPriority.php',
     'ManiphestTaskPriorityDatasource' => 'applications/maniphest/typeahead/ManiphestTaskPriorityDatasource.php',
     'ManiphestTaskPriorityHeraldAction' => 'applications/maniphest/herald/ManiphestTaskPriorityHeraldAction.php',
@@ -2206,6 +2208,7 @@ phutil_register_library_map(array(
     'PhabricatorEditEngineExtension' => 'applications/transactions/engineextension/PhabricatorEditEngineExtension.php',
     'PhabricatorEditEngineExtensionModule' => 'applications/transactions/engineextension/PhabricatorEditEngineExtensionModule.php',
     'PhabricatorEditEngineListController' => 'applications/transactions/controller/PhabricatorEditEngineListController.php',
+    'PhabricatorEditEnginePointsCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEnginePointsCommentAction.php',
     'PhabricatorEditEngineQuery' => 'applications/transactions/query/PhabricatorEditEngineQuery.php',
     'PhabricatorEditEngineSearchEngine' => 'applications/transactions/query/PhabricatorEditEngineSearchEngine.php',
     'PhabricatorEditEngineSelectCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEngineSelectCommentAction.php',
@@ -2813,6 +2816,7 @@ phutil_register_library_map(array(
     'PhabricatorPhurlURLViewController' => 'applications/phurl/controller/PhabricatorPhurlURLViewController.php',
     'PhabricatorPirateEnglishTranslation' => 'infrastructure/internationalization/translation/PhabricatorPirateEnglishTranslation.php',
     'PhabricatorPlatformSite' => 'aphront/site/PhabricatorPlatformSite.php',
+    'PhabricatorPointsEditField' => 'applications/transactions/editfield/PhabricatorPointsEditField.php',
     'PhabricatorPolicies' => 'applications/policy/constants/PhabricatorPolicies.php',
     'PhabricatorPolicy' => 'applications/policy/storage/PhabricatorPolicy.php',
     'PhabricatorPolicyApplication' => 'applications/policy/application/PhabricatorPolicyApplication.php',
@@ -4226,7 +4230,7 @@ phutil_register_library_map(array(
     'ConduitGetCapabilitiesConduitAPIMethod' => 'ConduitAPIMethod',
     'ConduitGetCertificateConduitAPIMethod' => 'ConduitAPIMethod',
     'ConduitIntListParameterType' => 'ConduitListParameterType',
-    'ConduitIntParameterType' => 'ConduitListParameterType',
+    'ConduitIntParameterType' => 'ConduitParameterType',
     'ConduitListParameterType' => 'ConduitParameterType',
     'ConduitLogGarbageCollector' => 'PhabricatorGarbageCollector',
     'ConduitMethodDoesNotExistException' => 'ConduitMethodNotFoundException',
@@ -4235,6 +4239,7 @@ phutil_register_library_map(array(
     'ConduitPHIDParameterType' => 'ConduitParameterType',
     'ConduitParameterType' => 'Phobject',
     'ConduitPingConduitAPIMethod' => 'ConduitAPIMethod',
+    'ConduitPointsParameterType' => 'ConduitParameterType',
     'ConduitProjectListParameterType' => 'ConduitListParameterType',
     'ConduitQueryConduitAPIMethod' => 'ConduitAPIMethod',
     'ConduitResultSearchEngineExtension' => 'PhabricatorSearchEngineExtension',
@@ -5508,6 +5513,7 @@ phutil_register_library_map(array(
     'ManiphestTaskOpenStatusDatasource' => 'PhabricatorTypeaheadDatasource',
     'ManiphestTaskPHIDResolver' => 'PhabricatorPHIDResolver',
     'ManiphestTaskPHIDType' => 'PhabricatorPHIDType',
+    'ManiphestTaskPoints' => 'Phobject',
     'ManiphestTaskPriority' => 'ManiphestConstants',
     'ManiphestTaskPriorityDatasource' => 'PhabricatorTypeaheadDatasource',
     'ManiphestTaskPriorityHeraldAction' => 'HeraldAction',
@@ -6508,6 +6514,7 @@ phutil_register_library_map(array(
     'PhabricatorEditEngineExtension' => 'Phobject',
     'PhabricatorEditEngineExtensionModule' => 'PhabricatorConfigModule',
     'PhabricatorEditEngineListController' => 'PhabricatorEditEngineController',
+    'PhabricatorEditEnginePointsCommentAction' => 'PhabricatorEditEngineCommentAction',
     'PhabricatorEditEngineQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PhabricatorEditEngineSearchEngine' => 'PhabricatorApplicationSearchEngine',
     'PhabricatorEditEngineSelectCommentAction' => 'PhabricatorEditEngineCommentAction',
@@ -7206,6 +7213,7 @@ phutil_register_library_map(array(
     'PhabricatorPhurlURLViewController' => 'PhabricatorPhurlController',
     'PhabricatorPirateEnglishTranslation' => 'PhutilTranslation',
     'PhabricatorPlatformSite' => 'PhabricatorSite',
+    'PhabricatorPointsEditField' => 'PhabricatorEditField',
     'PhabricatorPolicies' => 'PhabricatorPolicyConstants',
     'PhabricatorPolicy' => array(
       'PhabricatorPolicyDAO',

src/applications/conduit/parametertype/ConduitIntParameterType.php

@@ -1,7 +1,7 @@
 <?php
 
 final class ConduitIntParameterType
-  extends ConduitListParameterType {
+  extends ConduitParameterType {
 
   protected function getParameterValue(array $request, $key) {
     $value = parent::getParameterValue($request, $key);

src/applications/conduit/parametertype/ConduitPointsParameterType.php

@@ -0,0 +1,49 @@
+<?php
+
+final class ConduitPointsParameterType
+  extends ConduitParameterType {
+
+  protected function getParameterValue(array $request, $key) {
+    $value = parent::getParameterValue($request, $key);
+
+    if (($value !== null) && !is_numeric($value)) {
+      $this->raiseValidationException(
+        $request,
+        $key,
+        pht('Expected numeric points value, got something else.'));
+    }
+
+    if ($value !== null) {
+      $value = (double)$value;
+      if ($value < 0) {
+        $this->raiseValidationException(
+          $request,
+          $key,
+          pht('Point values must be nonnegative.'));
+      }
+    }
+
+    return $value;
+  }
+
+  protected function getParameterTypeName() {
+    return 'points';
+  }
+
+  protected function getParameterFormatDescriptions() {
+    return array(
+      pht('A nonnegative number, or null.'),
+    );
+  }
+
+  protected function getParameterExamples() {
+    return array(
+      'null',
+      '0',
+      '1',
+      '15',
+      '0.5',
+    );
+  }
+
+}

src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php

@@ -336,7 +336,9 @@ EOTEXT
             '"Needs Triage" panel on the home page. You should adjust this if '.
             'you adjust priorities using `%s`.',
             'maniphest.priorities')),
-
+      $this->newOption('maniphest.points', 'map<string, wild>', array())
+        ->setDescription(
+          pht('PROTOTYPE! Very hot. Burns user. Do not touch!')),
     );
   }
 

src/applications/maniphest/constants/ManiphestTaskPoints.php

@@ -0,0 +1,24 @@
+<?php
+
+final class ManiphestTaskPoints extends Phobject {
+
+  public static function getIsEnabled() {
+    $config = self::getPointsConfig();
+    return idx($config, 'enabled');
+  }
+
+  public static function getPointsLabel() {
+    $config = self::getPointsConfig();
+    return idx($config, 'label', pht('Points'));
+  }
+
+  public static function getPointsActionLabel() {
+    $config = self::getPointsConfig();
+    return idx($config, 'action', pht('Change Points'));
+  }
+
+  private static function getPointsConfig() {
+    return PhabricatorEnv::getEnvConfig('maniphest.points');
+  }
+
+}

src/applications/maniphest/editor/ManiphestEditEngine.php

@@ -77,7 +77,7 @@ final class ManiphestEditEngine
       $owner_value = array($this->getViewer()->getPHID());
     }
 
-    return array(
+    $fields = array(
       id(new PhabricatorHandlesEditField())
         ->setKey('parent')
         ->setLabel(pht('Parent Task'))
@@ -149,18 +149,37 @@ final class ManiphestEditEngine
         ->setValue($object->getPriority())
         ->setOptions($priority_map)
         ->setCommentActionLabel(pht('Change Priority')),
-      id(new PhabricatorRemarkupEditField())
-        ->setKey('description')
-        ->setLabel(pht('Description'))
-        ->setDescription(pht('Task description.'))
-        ->setConduitDescription(pht('Update the task description.'))
-        ->setConduitTypeDescription(pht('New task description.'))
-        ->setTransactionType(ManiphestTransaction::TYPE_DESCRIPTION)
-        ->setValue($object->getDescription())
-        ->setPreviewPanel(
-          id(new PHUIRemarkupPreviewPanel())
-            ->setHeader(pht('Description Preview'))),
     );
+
+    if (ManiphestTaskPoints::getIsEnabled()) {
+      $points_label = ManiphestTaskPoints::getPointsLabel();
+      $action_label = ManiphestTaskPoints::getPointsActionLabel();
+
+      $fields[] = id(new PhabricatorPointsEditField())
+        ->setKey('points')
+        ->setLabel($points_label)
+        ->setDescription(pht('Point value of the task.'))
+        ->setConduitDescription(pht('Change the task point value.'))
+        ->setConduitTypeDescription(pht('New task point value.'))
+        ->setTransactionType(ManiphestTransaction::TYPE_POINTS)
+        ->setIsCopyable(true)
+        ->setValue($object->getPoints())
+        ->setCommentActionLabel($action_label);
+    }
+
+    $fields[] = id(new PhabricatorRemarkupEditField())
+      ->setKey('description')
+      ->setLabel(pht('Description'))
+      ->setDescription(pht('Task description.'))
+      ->setConduitDescription(pht('Update the task description.'))
+      ->setConduitTypeDescription(pht('New task description.'))
+      ->setTransactionType(ManiphestTransaction::TYPE_DESCRIPTION)
+      ->setValue($object->getDescription())
+      ->setPreviewPanel(
+        id(new PHUIRemarkupPreviewPanel())
+          ->setHeader(pht('Description Preview')));
+
+    return $fields;
   }
 
   private function getTaskStatusMap(ManiphestTask $task) {

src/applications/maniphest/editor/ManiphestTransactionEditor.php

@@ -29,6 +29,7 @@ final class ManiphestTransactionEditor
     $types[] = ManiphestTransaction::TYPE_PARENT;
     $types[] = ManiphestTransaction::TYPE_COLUMN;
     $types[] = ManiphestTransaction::TYPE_COVER_IMAGE;
+    $types[] = ManiphestTransaction::TYPE_POINTS;
     $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
     $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
 
@@ -69,6 +70,8 @@ final class ManiphestTransactionEditor
         return $object->getSubpriority();
       case ManiphestTransaction::TYPE_COVER_IMAGE:
         return $object->getCoverImageFilePHID();
+      case ManiphestTransaction::TYPE_POINTS:
+        return $object->getPoints();
       case ManiphestTransaction::TYPE_MERGED_INTO:
       case ManiphestTransaction::TYPE_MERGED_FROM:
         return null;
@@ -100,6 +103,15 @@ final class ManiphestTransactionEditor
       case ManiphestTransaction::TYPE_PARENT:
       case ManiphestTransaction::TYPE_COLUMN:
         return $xaction->getNewValue();
+      case ManiphestTransaction::TYPE_POINTS:
+        $value = $xaction->getNewValue();
+        if (!strlen($value)) {
+          $value = null;
+        }
+        if ($value !== null) {
+          $value = (double)$value;
+        }
+        return $value;
     }
   }
 
@@ -191,6 +203,9 @@ final class ManiphestTransactionEditor
         $object->setProperty('cover.filePHID', $file->getPHID());
         $object->setProperty('cover.thumbnailPHID', $xform->getPHID());
         return;
+      case ManiphestTransaction::TYPE_POINTS:
+        $object->setPoints($xaction->getNewValue());
+        return;
       case ManiphestTransaction::TYPE_MERGED_FROM:
       case ManiphestTransaction::TYPE_PARENT:
       case ManiphestTransaction::TYPE_COLUMN:
@@ -884,6 +899,30 @@ final class ManiphestTransactionEditor
           }
         }
         break;
+
+      case ManiphestTransaction::TYPE_POINTS:
+        foreach ($xactions as $xaction) {
+          $new = $xaction->getNewValue();
+          if (strlen($new) && !is_numeric($new)) {
+            $errors[] = new PhabricatorApplicationTransactionValidationError(
+              $type,
+              pht('Invalid'),
+              pht('Points value must be numeric or empty.'),
+              $xaction);
+            continue;
+          }
+
+          if ((double)$new < 0) {
+            $errors[] = new PhabricatorApplicationTransactionValidationError(
+              $type,
+              pht('Invalid'),
+              pht('Points value must be nonnegative.'),
+              $xaction);
+            continue;
+          }
+        }
+        break;
+
     }
 
     return $errors;

src/applications/maniphest/storage/ManiphestTransaction.php

@@ -17,6 +17,7 @@ final class ManiphestTransaction
   const TYPE_PARENT = 'parent';
   const TYPE_COLUMN = 'column';
   const TYPE_COVER_IMAGE = 'cover-image';
+  const TYPE_POINTS = 'points';
 
   // NOTE: this type is deprecated. Keep it around for legacy installs
   // so any transactions render correctly.
@@ -166,11 +167,24 @@ final class ManiphestTransaction
       case self::TYPE_COVER_IMAGE:
         // At least for now, don't show these.
         return true;
+      case self::TYPE_POINTS:
+        if (!ManiphestTaskPoints::getIsEnabled()) {
+          return true;
+        }
     }
 
     return parent::shouldHide();
   }
 
+  public function shouldHideForMail(array $xactions) {
+    switch ($this->getTransactionType()) {
+      case self::TYPE_POINTS:
+        return true;
+    }
+
+    return parent::shouldHideForMail();
+  }
+
   public function shouldHideForFeed() {
     switch ($this->getTransactionType()) {
       case self::TYPE_UNBLOCK:
@@ -181,6 +195,8 @@ final class ManiphestTransaction
           return true;
         }
         break;
+      case self::TYPE_POINTS:
+        return true;
     }
 
     return parent::shouldHideForFeed();
@@ -624,6 +640,23 @@ final class ManiphestTransaction
           $this->renderHandleList($new));
         break;
 
+      case self::TYPE_POINTS:
+        if ($old === null) {
+          return pht(
+            '%s set the point value for this task to %s.',
+            $this->renderHandleLink($author_phid),
+            $new);
+        } else if ($new === null) {
+          return pht(
+            '%s removed the point value for this task.',
+            $this->renderHandleLink($author_phid));
+        } else {
+          return pht(
+            '%s changed the point value for this task from %s to %s.',
+            $this->renderHandleLink($author_phid),
+            $old,
+            $new);
+        }
 
     }
 

src/applications/transactions/commentaction/PhabricatorEditEnginePointsCommentAction.php

@@ -0,0 +1,16 @@
+<?php
+
+final class PhabricatorEditEnginePointsCommentAction
+  extends PhabricatorEditEngineCommentAction {
+
+  public function getPHUIXControlType() {
+    return 'points';
+  }
+
+  public function getPHUIXControlSpecification() {
+    return array(
+      'value' => $this->getValue(),
+    );
+  }
+
+}

src/applications/transactions/editfield/PhabricatorPointsEditField.php

@@ -0,0 +1,17 @@
+<?php
+
+final class PhabricatorPointsEditField
+  extends PhabricatorEditField {
+
+  protected function newControl() {
+    return new AphrontFormTextControl();
+  }
+
+  protected function newConduitParameterType() {
+    return new ConduitPointsParameterType();
+  }
+
+  protected function newCommentAction() {
+    return id(new PhabricatorEditEnginePointsCommentAction());
+  }
+}

webroot/rsrc/js/phuix/PHUIXFormControl.js

@@ -35,6 +35,9 @@ JX.install('PHUIXFormControl', {
         case 'select':
           input = this._newSelect(spec);
           break;
+        case 'points':
+          input = this._newPoints(spec);
+          break;
         default:
           // TODO: Default or better error?
           JX.$E('Bad Input Type');
@@ -149,6 +152,25 @@ JX.install('PHUIXFormControl', {
         {},
         spec.order);
 
+      return {
+        node: node,
+        get: function() {
+          return node.value;
+        },
+        set: function(value) {
+          node.value = value;
+        }
+      };
+    },
+
+    _newPoints: function(spec) {
+      var attrs = {
+        type: 'text',
+        value: spec.value
+      };
+
+      var node = JX.$N('input', attrs);
+
       return {
         node: node,
         get: function() {