Move object-selector closable to being usable.

Summary:

Test Plan:

Reviewers:

CC:

src/__celerity_resource_map__.php

@@ -18,7 +18,7 @@ celerity_register_resource_map(array(
   ),
   'aphront-dialog-view-css' =>
   array(
-    'uri' => '/res/a05107ae/rsrc/css/aphront/dialog-view.css',
+    'uri' => '/res/c8324e86/rsrc/css/aphront/dialog-view.css',
     'type' => 'css',
     'requires' =>
     array(
@@ -217,7 +217,7 @@ celerity_register_resource_map(array(
   ),
   'phabricator-object-selector-css' =>
   array(
-    'uri' => '/res/270ce107/rsrc/css/application/objectselector/object-selector.css',
+    'uri' => '/res/52a7e289/rsrc/css/application/objectselector/object-selector.css',
     'type' => 'css',
     'requires' =>
     array(
@@ -272,7 +272,7 @@ celerity_register_resource_map(array(
   ),
   'javelin-behavior-phabricator-object-selector' =>
   array(
-    'uri' => '/res/7f7eda6a/rsrc/js/application/core/behavior-object-selector.js',
+    'uri' => '/res/e849ced6/rsrc/js/application/core/behavior-object-selector.js',
     'type' => 'js',
     'requires' =>
     array(
@@ -455,7 +455,7 @@ celerity_register_resource_map(array(
 ), array (
   'packages' =>
   array (
-    '4f907a28' =>
+    'aa43d409' =>
     array (
       'name' => 'core.pkg.css',
       'symbols' =>
@@ -474,7 +474,7 @@ celerity_register_resource_map(array(
         11 => 'phabricator-remarkup-css',
         12 => 'syntax-highlighting-css',
       ),
-      'uri' => '/res/pkg/4f907a28/core.pkg.css',
+      'uri' => '/res/pkg/aa43d409/core.pkg.css',
       'type' => 'css',
     ),
     '2525bbc7' =>
@@ -511,19 +511,19 @@ celerity_register_resource_map(array(
   ),
   'reverse' =>
   array (
-    'phabricator-core-css' => '4f907a28',
+    'phabricator-core-css' => 'aa43d409',
-    'phabricator-core-buttons-css' => '4f907a28',
+    'phabricator-core-buttons-css' => 'aa43d409',
-    'phabricator-standard-page-view' => '4f907a28',
+    'phabricator-standard-page-view' => 'aa43d409',
-    'aphront-dialog-view-css' => '4f907a28',
+    'aphront-dialog-view-css' => 'aa43d409',
-    'aphront-form-view-css' => '4f907a28',
+    'aphront-form-view-css' => 'aa43d409',
-    'aphront-panel-view-css' => '4f907a28',
+    'aphront-panel-view-css' => 'aa43d409',
-    'aphront-side-nav-view-css' => '4f907a28',
+    'aphront-side-nav-view-css' => 'aa43d409',
-    'aphront-table-view-css' => '4f907a28',
+    'aphront-table-view-css' => 'aa43d409',
-    'aphront-tokenizer-control-css' => '4f907a28',
+    'aphront-tokenizer-control-css' => 'aa43d409',
-    'aphront-typeahead-control-css' => '4f907a28',
+    'aphront-typeahead-control-css' => 'aa43d409',
-    'phabricator-directory-css' => '4f907a28',
+    'phabricator-directory-css' => 'aa43d409',
-    'phabricator-remarkup-css' => '4f907a28',
+    'phabricator-remarkup-css' => 'aa43d409',
-    'syntax-highlighting-css' => '4f907a28',
+    'syntax-highlighting-css' => 'aa43d409',
     'differential-core-view-css' => '2525bbc7',
     'differential-changeset-view-css' => '2525bbc7',
     'differential-revision-detail-css' => '2525bbc7',

src/__phutil_library_map__.php

@@ -89,6 +89,7 @@ phutil_register_library_map(array(
     'DarkConsoleXHProfPluginAPI' => 'aphront/console/plugin/xhprof/api',
     'DifferentialAction' => 'applications/differential/constants/action',
     'DifferentialAddCommentView' => 'applications/differential/view/addcomment',
+    'DifferentialAttachController' => 'applications/differential/controller/attach',
     'DifferentialCCWelcomeMail' => 'applications/differential/mail/ccwelcome',
     'DifferentialChangeType' => 'applications/differential/constants/changetype',
     'DifferentialChangeset' => 'applications/differential/storage/changeset',
@@ -191,6 +192,7 @@ phutil_register_library_map(array(
     'PhabricatorFileURI' => 'applications/files/uri',
     'PhabricatorFileUploadController' => 'applications/files/controller/upload',
     'PhabricatorFileViewController' => 'applications/files/controller/view',
+    'PhabricatorHandleObjectSelectorDataView' => 'applications/phid/handle/view/selector',
     'PhabricatorLiskDAO' => 'applications/base/storage/lisk',
     'PhabricatorLoginController' => 'applications/auth/controller/login',
     'PhabricatorLogoutController' => 'applications/auth/controller/logout',
@@ -341,6 +343,7 @@ phutil_register_library_map(array(
     'DarkConsoleServicesPlugin' => 'DarkConsolePlugin',
     'DarkConsoleXHProfPlugin' => 'DarkConsolePlugin',
     'DifferentialAddCommentView' => 'AphrontView',
+    'DifferentialAttachController' => 'DifferentialController',
     'DifferentialCCWelcomeMail' => 'DifferentialReviewRequestMail',
     'DifferentialChangeset' => 'DifferentialDAO',
     'DifferentialChangesetDetailView' => 'AphrontView',
@@ -435,7 +438,6 @@ phutil_register_library_map(array(
     'PhabricatorMetaMTAMailingListsController' => 'PhabricatorMetaMTAController',
     'PhabricatorMetaMTASendController' => 'PhabricatorMetaMTAController',
     'PhabricatorMetaMTAViewController' => 'PhabricatorMetaMTAController',
-    'PhabricatorObjectSelectorDialog' => 'AphrontDialogView',
     'PhabricatorPHID' => 'PhabricatorPHIDDAO',
     'PhabricatorPHIDAllocateController' => 'PhabricatorPHIDController',
     'PhabricatorPHIDController' => 'PhabricatorController',

src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php

@@ -92,6 +92,7 @@ class AphrontDefaultApplicationConfiguration
             'edit/(?P<id>\d+)/$' => 'DifferentialInlineCommentEditController',
           ),
         ),
+        'attach/(?P<id>\d+)/(?P<type>\w+)/$' => 'DifferentialAttachController',
       ),
 
       '/res/' => array(
@@ -183,6 +184,21 @@ class AphrontDefaultApplicationConfiguration
         '<code>'.phutil_escape_html((string)$ex).'</code>'.
       '</div>';
 
+    if ($this->getRequest()->isAjax()) {
+      $dialog = new AphrontDialogView();
+      $dialog
+        ->setTitle('Exception!')
+        ->setClass('aphront-exception-dialog')
+        ->setUser($this->getRequest()->getUser())
+        ->appendChild($content)
+        ->addCancelButton('/');
+
+      $response = new AphrontDialogResponse();
+      $response->setDialog($dialog);
+
+      return $response;
+    }
+
     $view = new PhabricatorStandardPageView();
     $view->setRequest($this->getRequest());
     $view->appendChild($content);

src/aphront/default/configuration/__init__.php

@@ -9,8 +9,10 @@
 phutil_require_module('phabricator', 'aphront/applicationconfiguration');
 phutil_require_module('phabricator', 'aphront/request');
 phutil_require_module('phabricator', 'aphront/response/ajax');
+phutil_require_module('phabricator', 'aphront/response/dialog');
 phutil_require_module('phabricator', 'aphront/response/webpage');
 phutil_require_module('phabricator', 'applications/base/controller/404');
+phutil_require_module('phabricator', 'view/dialog');
 phutil_require_module('phabricator', 'view/page/failure');
 phutil_require_module('phabricator', 'view/page/standard');
 

src/applications/differential/controller/attach/DifferentialAttachController.php

@@ -0,0 +1,117 @@
+<?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.
+ */
+
+class DifferentialAttachController extends DifferentialController {
+
+  private $id;
+  private $type;
+
+  public function willProcessRequest(array $data) {
+    $this->id = $data['id'];
+    $this->type = $data['type'];
+  }
+
+  public function processRequest() {
+
+    $request = $this->getRequest();
+    $user = $request->getUser();
+
+    $revision = id(new DifferentialRevision())->load($this->id);
+    if (!$revision) {
+      return new Aphront404Response();
+    }
+
+    if ($request->isFormPost()) {
+      $phids = explode(';', $request->getStr('phids'));
+      $old_phids = $revision->getAttachedPHIDs('TASK');
+
+      if (($phids || $old_phids) && ($phids != $old_phids)) {
+        $tasks = id(new ManiphestTask())->loadAllWhere(
+          'phid in (%Ls)',
+          array_merge($phids, $old_phids));
+        $tasks = mpull($tasks, null, 'getPHID');
+
+        // Remove PHIDs which don't actually exist.
+        $phids = array_keys(array_select_keys($tasks, $phids));
+
+        $revision->setAttachedPHIDs($this->type, $phids);
+        $revision->save();
+
+        $editor = new ManiphestTransactionEditor();
+        $type = ManiphestTransactionType::TYPE_ATTACH;
+        foreach ($tasks as $task) {
+          $transaction = new ManiphestTransaction();
+          $transaction->setAuthorPHID($user->getPHID());
+          $transaction->setTransactionType($type);
+          $new = $task->getAttached();
+          if (empty($new['DREV'])) {
+            $new['DREV'] = array();
+          }
+          $rev_phid = $revision->getPHID();
+          if (in_array($task->getPHID(), $phids)) {
+            if (in_array($rev_phid, $task->getAttachedPHIDs('DREV'))) {
+              // TODO: maybe the transaction editor should be responsible for
+              // this?
+              continue;
+            }
+            $new['DREV'][$rev_phid] = array();
+          } else {
+            if (!in_array($rev_phid, $task->getAttachedPHIDs('DREV'))) {
+              continue;
+            }
+            unset($new['DREV'][$rev_phid]);
+          }
+          $transaction->setNewValue($new);
+          $editor->applyTransactions($task, array($transaction));
+        }
+      }
+
+      if ($request->isAjax()) {
+        return id(new AphrontRedirectResponse());
+      } else {
+        return id(new AphrontRedirectResponse())
+          ->setURI('/D'.$revision->getID());
+      }
+    } else {
+      $phids = $revision->getAttachedPHIDs($this->type);
+    }
+
+
+    $handles = id(new PhabricatorObjectHandleData($phids))
+      ->loadHandles();
+
+    $obj_dialog = new PhabricatorObjectSelectorDialog();
+    $obj_dialog
+      ->setUser($user)
+      ->setHandles($handles)
+      ->setFilters(array(
+        'assigned' => 'Assigned to Me',
+        'created'  => 'Created By Me',
+        'open'     => 'All Open Tasks',
+        'all'      => 'All Tasks',
+      ))
+      ->setCancelURI('#')
+      ->setSearchURI('/maniphest/select/search/')
+      ->setNoun('Tasks');
+
+    $dialog = $obj_dialog->buildDialog();
+
+    return id(new AphrontDialogResponse())->setDialog($dialog);
+  }
+
+}

src/applications/differential/controller/attach/__init__.php

@@ -0,0 +1,24 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'aphront/response/404');
+phutil_require_module('phabricator', 'aphront/response/dialog');
+phutil_require_module('phabricator', 'aphront/response/redirect');
+phutil_require_module('phabricator', 'applications/differential/controller/base');
+phutil_require_module('phabricator', 'applications/differential/storage/revision');
+phutil_require_module('phabricator', 'applications/maniphest/constants/transactiontype');
+phutil_require_module('phabricator', 'applications/maniphest/editor/transaction');
+phutil_require_module('phabricator', 'applications/maniphest/storage/task');
+phutil_require_module('phabricator', 'applications/maniphest/storage/transaction');
+phutil_require_module('phabricator', 'applications/phid/handle/data');
+phutil_require_module('phabricator', 'view/control/objectselector');
+
+phutil_require_module('phutil', 'utils');
+
+
+phutil_require_source('DifferentialAttachController.php');

src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php

@@ -64,6 +64,11 @@ class DifferentialRevisionViewController extends DifferentialController {
         $user->getPHID(),
       ),
       mpull($comments, 'getAuthorPHID'));
+    foreach ($revision->getAttached() as $type => $phids) {
+      foreach ($phids as $phid => $info) {
+        $object_phids[] = $phid;
+      }
+    }
     $object_phids = array_unique($object_phids);
 
     $handles = id(new PhabricatorObjectHandleData($object_phids))
@@ -229,6 +234,15 @@ class DifferentialRevisionViewController extends DifferentialController {
     $umsg = DifferentialRevisionUpdateHistoryView::getDiffUnitMessage($diff);
     $properties['Unit'] = $ustar.' '.$umsg;
 
+    $tasks = $revision->getAttachedPHIDs('TASK');
+    if ($tasks) {
+      $links = array();
+      foreach ($tasks as $task_phid) {
+        $links[] = $handles[$task_phid]->renderLink();
+      }
+      $properties['Maniphest Tasks'] = implode('<br />', $links);
+    }
+
     return $properties;
   }
 
@@ -265,6 +279,16 @@ class DifferentialRevisionViewController extends DifferentialController {
       );
     }
 
+    require_celerity_resource('phabricator-object-selector-css');
+    require_celerity_resource('javelin-behavior-phabricator-object-selector');
+
+    $links[] = array(
+      'class' => 'attach-maniphest',
+      'name'  => 'Edit Maniphest Tasks',
+      'href'  => "/differential/attach/{$revision_id}/TASK/",
+      'sigil' => 'workflow',
+    );
+
     $links[] = array(
       'class' => 'transcripts-metamta',
       'name'  => 'MetaMTA Transcripts',

src/applications/differential/controller/revisionview/__init__.php

@@ -22,6 +22,7 @@ phutil_require_module('phabricator', 'applications/differential/view/revisiondet
 phutil_require_module('phabricator', 'applications/differential/view/revisionupdatehistory');
 phutil_require_module('phabricator', 'applications/draft/storage/draft');
 phutil_require_module('phabricator', 'applications/phid/handle/data');
+phutil_require_module('phabricator', 'infrastructure/celerity/api');
 phutil_require_module('phabricator', 'view/form/error');
 
 phutil_require_module('phutil', 'markup');

src/applications/differential/storage/revision/DifferentialRevision.php

@@ -32,6 +32,7 @@ class DifferentialRevision extends DifferentialDAO {
   protected $dateCommitted;
 
   protected $lineCount;
+  protected $attached = array();
 
   private $relationships;
 
@@ -44,9 +45,21 @@ class DifferentialRevision extends DifferentialDAO {
   public function getConfiguration() {
     return array(
       self::CONFIG_AUX_PHID => true,
+      self::CONFIG_SERIALIZATION => array(
+        'attached' => self::SERIALIZATION_JSON,
+      ),
     ) + parent::getConfiguration();
   }
 
+  public function getAttachedPHIDs($type) {
+    return array_keys(idx($this->attached, $type, array()));
+  }
+
+  public function setAttachedPHIDs($type, array $phids) {
+    $this->attached[$type] = array_fill_keys($phids, array());
+    return $this;
+  }
+
   public function generatePHID() {
     return PhabricatorPHID::generateNewPHID('DREV');
   }

src/applications/differential/view/revisiondetail/DifferentialRevisionDetailView.php

@@ -65,13 +65,12 @@ final class DifferentialRevisionDetailView extends AphrontView {
       } else {
         $tag = 'a';
       }
-      $actions[] = phutil_render_tag(
+      $name = $action['name'];
+      unset($action['name']);
+      $actions[] = javelin_render_tag(
         $tag,
-        array(
+        $action,
-          'href'  => idx($action, 'href'),
+        phutil_escape_html($name));
-          'class' => idx($action, 'class'),
-        ),
-        phutil_escape_html($action['name']));
     }
     $actions = implode("\n", $actions);
 

src/applications/differential/view/revisiondetail/__init__.php

@@ -7,10 +7,10 @@
 
 
 phutil_require_module('phabricator', 'infrastructure/celerity/api');
+phutil_require_module('phabricator', 'infrastructure/javelin/markup');
 phutil_require_module('phabricator', 'view/base');
 
 phutil_require_module('phutil', 'markup');
-phutil_require_module('phutil', 'utils');
 
 
 phutil_require_source('DifferentialRevisionDetailView.php');

src/applications/maniphest/constants/transactiontype/ManiphestTransactionType.php

@@ -24,6 +24,8 @@ final class ManiphestTransactionType {
   const TYPE_CCS          = 'ccs';
   const TYPE_PRIORITY     = 'priority';
 
+  const TYPE_ATTACH       = 'attach';
+
   public static function getTransactionTypeMap() {
     return array(
       self::TYPE_NONE       => 'Comment',

src/applications/maniphest/controller/taskdetail/ManiphestTaskDetailController.php

@@ -54,6 +54,13 @@ class ManiphestTaskDetailController extends ManiphestController {
     $phids[$task->getAuthorPHID()] = true;
     $phids = array_keys($phids);
 
+    $attached = $task->getAttached();
+    foreach ($attached as $type => $list) {
+      foreach ($list as $phid => $info) {
+        $phids[$phid] = true;
+      }
+    }
+
     $handles = id(new PhabricatorObjectHandleData($phids))
       ->loadHandles();
 
@@ -86,6 +93,16 @@ class ManiphestTaskDetailController extends ManiphestController {
 
     $dict['Author'] = $handles[$task->getAuthorPHID()]->renderLink();
 
+    if (idx($attached, 'DREV')) {
+      $revs = idx($attached, 'DREV');
+      $rev_links = array();
+      foreach ($revs as $rev => $info) {
+        $rev_links[] = $handles[$rev]->renderLink();
+      }
+      $rev_links = implode(', ', $rev_links);
+      $dict['Revisions'] = $rev_links;
+    }
+
     $dict['Description'] =
       '<div class="maniphest-task-description">'.
         '<div class="phabricator-remarkup">'.

src/applications/maniphest/controller/taskselector/ManiphestTaskSelectorController.php

@@ -22,142 +22,29 @@ class ManiphestTaskSelectorController extends ManiphestController {
     $request = $this->getRequest();
     $user = $request->getUser();
 
-    $filter_id = celerity_generate_unique_node_id();
+    $phids = $request->getArr('phids');
-    $query_id = celerity_generate_unique_node_id();
-    $search_id = celerity_generate_unique_node_id();
-    $results_id = celerity_generate_unique_node_id();
-    $current_id = celerity_generate_unique_node_id();
 
-    $search_box =
+    $handles = id(new PhabricatorObjectHandleData($phids))
-      '<table class="phabricator-object-selector-search">
+      ->loadHandles();
-        <tr>
-          <td class="phabricator-object-selector-search-filter">
-            <select id="'.$filter_id.'">
-              <option>Assigned To Me</option>
-              <option>Created By Me</option>
-              <option>All Open Tasks</option>
-              <option>All Tasks</option>
-            </select>
-          </td>
-          <td class="phabricator-object-selector-search-text">
-            <input type="text" id="'.$query_id.'" />
-          </td>
-          <td class="phabricator-object-selector-search-button">
-            <a href="#" class="button" id="'.$search_id.'">Search</a>
-          </td>
-        </tr>
-      </table>';
-    $result_box =
-      '<div class="phabricator-object-selector-results" id="'.$results_id.'">'.
 
-      '</div>';
+    $obj_dialog = new PhabricatorObjectSelectorDialog();
-    $attached_box =
+    $obj_dialog
-      '<div class="phabricator-object-selector-current">'.
-        '<div class="phabricator-object-selector-currently-attached">'.
-          '<div class="phabricator-object-selector-header">'.
-            'Currently Attached Tasks'.
-          '</div>'.
-          '<div id="'.$current_id.'">'.
-          '</div>'.
-        '</div>'.
-      '</div>';
-
-    require_celerity_resource('phabricator-object-selector-css');
-
-    Javelin::initBehavior(
-      'phabricator-object-selector',
-      array(
-        'filter'  => $filter_id,
-        'query'   => $query_id,
-        'search'  => $search_id,
-        'results' => $results_id,
-        'current' => $current_id,
-        'uri'     => '/maniphest/select/search/',
-      ));
-
-    $dialog = new PhabricatorObjectSelectorDialog();
-    $dialog
       ->setUser($user)
-      ->setTitle('Manage Attached Tasks')
+      ->setHandles($handles)
-      ->setClass('phabricator-object-selector-dialog')
+      ->setFilters(array(
-      ->appendChild($search_box)
+        'assigned' => 'Assigned to Me',
-      ->appendChild($result_box)
+        'created'  => 'Created By Me',
-      ->appendChild($attached_box)
+        'open'     => 'All Open Tasks',
-      ->addCancelButton('#')
+        'all'      => 'All Tasks',
-      ->addSubmitButton('Save Tasks');
+      ))
+      ->setCancelURI('#')
+      ->setSearchURI('/maniphest/select/search/')
+      ->setNoun('Tasks');
+
+    $dialog = $obj_dialog->buildDialog();
+
 
     return id(new AphrontDialogResponse())->setDialog($dialog);
   }
 
 }
-
-/*
-
-        '<table class="phabricator-object-selector-handle">
-          <tr>
-            <th>
-              <input type="checkbox" />
-            </th>
-            <td>
-              <a href="#">T20: Internet Attack Internets</a>
-            </td>
-          </tr>
-        </table>'.
-        '<table class="phabricator-object-selector-handle">
-          <tr>
-            <th>
-              <input type="checkbox" />
-            </th>
-            <td>
-              <a href="#">T21: Internet Attack Internets</a>
-            </td>
-          </tr>
-        </table>'.
-        '<table class="phabricator-object-selector-handle">
-          <tr>
-            <th>
-              <input type="checkbox" />
-            </th>
-            <td>
-              <a href="#">T22: Internet Attack Internets</a>
-            </td>
-          </tr>
-        </table>'.
-        'more results<br />'.
-        'more results<br />'.
-        'more results<br />'.
-        'more results<br />'.
-        'more results<br />'.
-        'more results<br />'.
-        'more results<br />'.
-        'more results<br />'.
-        'more results<br />'.
-        'more results<br />'.
-
-*/
-
-
-/*
-
-          '<table class="phabricator-object-selector-handle">
-            <tr>
-              <th>
-                <input type="checkbox" />
-              </th>
-              <td>
-                <a href="#">T22: Internet Attack Internets</a>
-              </td>
-            </tr>
-          </table>'.
-          '<table class="phabricator-object-selector-handle">
-            <tr>
-              <th>
-                <input type="checkbox" />
-              </th>
-              <td>
-                <a href="#">T22: Internet Attack Internets</a>
-              </td>
-            </tr>
-          </table>'.
-
-*/

src/applications/maniphest/controller/taskselector/__init__.php

@@ -8,8 +8,7 @@
 
 phutil_require_module('phabricator', 'aphront/response/dialog');
 phutil_require_module('phabricator', 'applications/maniphest/controller/base');
-phutil_require_module('phabricator', 'infrastructure/celerity/api');
+phutil_require_module('phabricator', 'applications/phid/handle/data');
-phutil_require_module('phabricator', 'infrastructure/javelin/api');
 phutil_require_module('phabricator', 'view/control/objectselector');
 
 phutil_require_module('phutil', 'utils');

src/applications/maniphest/controller/taskselectorsearch/ManiphestTaskSelectorSearchController.php

@@ -28,14 +28,15 @@ class ManiphestTaskSelectorSearchController extends ManiphestController {
 
     $exec = new PhabricatorSearchMySQLExecutor();
     $results = $exec->executeSearch($query);
+    $results = ipull($results, 'phid');
+
+    $handles = id(new PhabricatorObjectHandleData($results))
+      ->loadHandles();
 
     $data = array();
-    foreach ($results as $result) {
+    foreach ($handles as $handle) {
-      $data[] = array(
+      $view = new PhabricatorHandleObjectSelectorDataView($handle);
-        'phid' => $result['phid'],
+      $data[] = $view->renderData();
-        'name' => $result['documentTitle'],
-        'href' => '#',
-      );
     }
 
     return id(new AphrontAjaxResponse())->setContent($data);

src/applications/maniphest/controller/taskselectorsearch/__init__.php

@@ -8,6 +8,8 @@
 
 phutil_require_module('phabricator', 'aphront/response/ajax');
 phutil_require_module('phabricator', 'applications/maniphest/controller/base');
+phutil_require_module('phabricator', 'applications/phid/handle/data');
+phutil_require_module('phabricator', 'applications/phid/handle/view/selector');
 phutil_require_module('phabricator', 'applications/search/execute/mysql');
 phutil_require_module('phabricator', 'applications/search/storage/query');
 

src/applications/maniphest/editor/transaction/ManiphestTransactionEditor.php

@@ -46,6 +46,9 @@ class ManiphestTransactionEditor {
         case ManiphestTransactionType::TYPE_PRIORITY:
           $old = $task->getPriority();
           break;
+        case ManiphestTransactionType::TYPE_ATTACH:
+          $old = $task->getAttached();
+          break;
         default:
           throw new Exception('Unknown action type.');
       }
@@ -78,6 +81,9 @@ class ManiphestTransactionEditor {
           case ManiphestTransactionType::TYPE_PRIORITY:
             $task->setPriority($new);
             break;
+          case ManiphestTransactionType::TYPE_ATTACH:
+            $task->setAttached($new);
+            break;
           default:
             throw new Exception('Unknown action type.');
         }
@@ -120,7 +126,6 @@ class ManiphestTransactionEditor {
     $handles = id(new PhabricatorObjectHandleData($phids))
       ->loadHandles();
 
-
     $view = new ManiphestTransactionDetailView();
     $view->setTransactionGroup($transactions);
     $view->setHandles($handles);

src/applications/maniphest/storage/task/ManiphestTask.php

@@ -29,18 +29,22 @@ class ManiphestTask extends ManiphestDAO {
   protected $title;
   protected $description;
 
-  protected $relatedPHIDs;
+  protected $attached = array();
 
   public function getConfiguration() {
     return array(
       self::CONFIG_AUX_PHID => true,
       self::CONFIG_SERIALIZATION => array(
         'ccPHIDs' => self::SERIALIZATION_JSON,
-        'relatedPHIDs' => self::SERIALIZATION_JSON,
+        'attached' => self::SERIALIZATION_JSON,
       ),
     ) + parent::getConfiguration();
   }
 
+  public function getAttachedPHIDs($type) {
+    return array_keys(idx($this->attached, $type, array()));
+  }
+
   public function generatePHID() {
     return PhabricatorPHID::generateNewPHID('TASK');
   }

src/applications/maniphest/storage/transaction/ManiphestTransaction.php

@@ -51,6 +51,22 @@ class ManiphestTransaction extends ManiphestDAO {
         $phids[] = $this->getOldValue();
         $phids[] = $this->getNewValue();
         break;
+      case ManiphestTransactionType::TYPE_ATTACH:
+        $old = $this->getOldValue();
+        $new = $this->getNewValue();
+        if (!is_array($old)) {
+          $old = array();
+        }
+        if (!is_array($new)) {
+          $new = array();
+        }
+        $val = array_merge(array_values($old), array_values($new));
+        foreach ($val as $stuff) {
+          foreach ($stuff as $phid => $ignored) {
+            $phids[] = $phid;
+          }
+        }
+        break;
     }
 
     $phids[] = $this->getAuthorPHID();

src/applications/maniphest/view/transactiondetail/ManiphestTransactionDetailView.php

@@ -104,9 +104,13 @@ class ManiphestTransactionDetailView extends AphrontView {
       $comments = $comment_transaction->getCache();
       if (!strlen($comments)) {
         $comments = $comment_transaction->getComments();
+        if (strlen($comments)) {
           $comments = $this->markupEngine->markupText($comments);
-        $transaction->setCache($comments);
+          $comment_transaction->setCache($comments);
-        $transaction->save();
+          if ($comment_transaction->getID()) {
+            $comment_transaction->save();
+          }
+        }
       }
       $comment_block =
         '<div class="maniphest-transaction-comments phabricator-remarkup">'.
@@ -225,8 +229,39 @@ class ManiphestTransactionDetailView extends AphrontView {
                   '"'.$new_name.'"';
         }
         break;
+      case ManiphestTransactionType::TYPE_ATTACH:
+        $old = nonempty($old, array());
+        $new = nonempty($new, array());
+
+        $old = array_keys(idx($old, 'DREV', array()));
+        $new = array_keys(idx($new, 'DREV', array()));
+        $added = array_diff($new, $old);
+        $removed = array_diff($old, $new);
+
+        $add_desc = $this->renderHandles($added);
+        $rem_desc = $this->renderHandles($removed);
+
+        if ($added && !$removed) {
+          $verb = 'Attached';
+          if (count($added) == 1) {
+            $desc = 'attached Differential Revision: '.$add_desc;
+          } else {
+            $desc = 'attached Differential Revisions: '.$add_desc;
+          }
+        } else if ($removed && !$added) {
+          $verb = 'Detached';
+          if (count($removed) == 1) {
+            $desc = 'detached Differential Revision: '.$rem_desc;
+          } else {
+            $desc = 'detached Differential Revisions: '.$rem_desc;
+          }
+        } else {
+          $desc = 'changed attached Differential Revisions, added: '.$add_desc.
+                                                         'removed: '.$rem_desc;
+        }
+        break;
       default:
-        return ' brazenly '.$type."'d";
+        return array($type, ' brazenly '.$type."'d", $classes);
     }
 
     return array($verb, $desc, $classes);

src/applications/phid/handle/PhabricatorObjectHandle.php

@@ -90,12 +90,21 @@ class PhabricatorObjectHandle {
   }
 
   public function renderLink() {
+
+    switch ($this->getType()) {
+      case 'USER':
+        $name = $this->getName();
+        break;
+      default:
+        $name = $this->getFullName();
+    }
+
     return phutil_render_tag(
       'a',
       array(
         'href' => $this->getURI(),
       ),
-      phutil_escape_html($this->getName()));
+      phutil_escape_html($name));
   }
 
 }

src/applications/phid/handle/data/PhabricatorObjectHandleData.php

@@ -115,6 +115,7 @@ class PhabricatorObjectHandleData {
               $handle->setType($type);
               $handle->setName($rev->getTitle());
               $handle->setURI('/D'.$rev->getID());
+              $handle->setFullName('D'.$rev->getID().': '.$rev->getTitle());
             }
             $handles[$phid] = $handle;
           }
@@ -138,6 +139,7 @@ class PhabricatorObjectHandleData {
               $handle->setType($type);
               $handle->setName($task->getTitle());
               $handle->setURI('/T'.$task->getID());
+              $handle->setFullName('T'.$task->getID().': '.$task->getTitle());
             }
             $handles[$phid] = $handle;
           }

src/applications/phid/handle/view/selector/PhabricatorHandleObjectSelectorDataView.php

@@ -0,0 +1,35 @@
+<?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.
+ */
+
+class PhabricatorHandleObjectSelectorDataView {
+
+  private $handle;
+
+  public function __construct($handle) {
+    $this->handle = $handle;
+  }
+
+  public function renderData() {
+    $handle = $this->handle;
+    return array(
+      'phid' => $handle->getPHID(),
+      'name' => $handle->getFullName(),
+      'href' => $handle->getURI(),
+    );
+  }
+}

src/applications/phid/handle/view/selector/__init__.php

@@ -0,0 +1,10 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+
+phutil_require_source('PhabricatorHandleObjectSelectorDataView.php');

src/storage/lisk/dao/LiskDAO.php

@@ -733,6 +733,10 @@ abstract class LiskDAO {
         $this->getID(),
         'version',
         $this->getVersion());
+      if ($conn->getAffectedRows() !== 1) {
+        throw new AphrontQueryObjectMissingException($use_locks);
+      }
+      $this->setVersion($this->getVersion() + 1);
     } else {
       $conn->query(
         'UPDATE %T SET %Q WHERE %C = %d',
@@ -740,14 +744,9 @@ abstract class LiskDAO {
         $map,
         $this->getIDKeyForUse(),
         $this->getID());
-    }
+      // We can't detect a missing object because updating an object without
-
+      // changing any values doesn't affect rows. We could jiggle timestamps
-    if ($conn->getAffectedRows() !== 1) {
+      // to catch this for objects which track them if we wanted.
-      throw new AphrontQueryObjectMissingException($use_locks);
-    }
-
-    if ($use_locks) {
-      $this->setVersion($this->getVersion() + 1);
     }
 
     $this->didWriteData();

src/view/control/objectselector/PhabricatorObjectSelectorDialog.php

@@ -16,7 +16,149 @@
  * limitations under the License.
  */
 
-class PhabricatorObjectSelectorDialog extends AphrontDialogView {
+class PhabricatorObjectSelectorDialog {
 
+  private $user;
+  private $filters = array();
+  private $handles = array();
+  private $cancelURI;
+  private $submitURI;
+  private $noun;
+  private $searchURI;
+
+  public function setUser($user) {
+    $this->user = $user;
+    return $this;
+  }
+
+  public function setFilters(array $filters) {
+    $this->filters = $filters;
+    return $this;
+  }
+
+  public function setHandles(array $handles) {
+    $this->handles = $handles;
+    return $this;
+  }
+
+  public function setCancelURI($cancel_uri) {
+    $this->cancelURI = $cancel_uri;
+    return $this;
+  }
+
+  public function setSubmitURI($submit_uri) {
+    $this->submitURI = $submit_uri;
+    return $this;
+  }
+
+  public function setSearchURI($search_uri) {
+    $this->searchURI = $search_uri;
+    return $this;
+  }
+
+  public function setNoun($noun) {
+    $this->noun = $noun;
+    return $this;
+  }
+
+  public function buildDialog() {
+    $user = $this->user;
+
+    $filter_id = celerity_generate_unique_node_id();
+    $query_id = celerity_generate_unique_node_id();
+    $results_id = celerity_generate_unique_node_id();
+    $current_id = celerity_generate_unique_node_id();
+    $search_id  = celerity_generate_unique_node_id();
+    $form_id = celerity_generate_unique_node_id();
+
+    require_celerity_resource('phabricator-object-selector-css');
+
+    $options = array();
+    foreach ($this->filters as $key => $label) {
+      $options[] = phutil_render_tag(
+        'option',
+        array(
+          'value' => $key
+        ),
+        $label);
+    }
+    $options = implode("\n", $options);
+
+    $search_box = phabricator_render_form(
+      $user,
+      array(
+        'method' => 'POST',
+        'action' => $this->submitURI,
+        'id'     => $search_id,
+      ),
+      '<table class="phabricator-object-selector-search">
+        <tr>
+          <td class="phabricator-object-selector-search-filter">
+            <select id="'.$filter_id.'">'.
+              $options.
+            '</select>
+          </td>
+          <td class="phabricator-object-selector-search-text">
+            <input type="text" id="'.$query_id.'" />
+          </td>
+          <td class="phabricator-object-selector-search-button">
+            <button>Search</button>
+          </td>
+        </tr>
+      </table>');
+    $result_box =
+      '<div class="phabricator-object-selector-results" id="'.$results_id.'">'.
+      '</div>';
+    $attached_box =
+      '<div class="phabricator-object-selector-current">'.
+        '<div class="phabricator-object-selector-currently-attached">'.
+          '<div class="phabricator-object-selector-header">'.
+            'Currently Attached '.$this->noun.
+          '</div>'.
+          '<div id="'.$current_id.'">'.
+          '</div>'.
+        '</div>'.
+      '</div>';
+
+
+    $dialog = new AphrontDialogView();
+    $dialog
+      ->setUser($this->user)
+      ->setTitle('Manage Attached '.$this->noun)
+      ->setClass('phabricator-object-selector-dialog')
+      ->appendChild($search_box)
+      ->appendChild($result_box)
+      ->appendChild($attached_box)
+      ->setRenderDialogAsDiv()
+      ->setFormID($form_id)
+      ->addSubmitButton('Save '.$this->noun);
+
+    if ($this->cancelURI) {
+      $dialog->addCancelButton($this->cancelURI);
+    }
+
+    $handle_views = array();
+    foreach ($this->handles as $phid => $handle) {
+      $view = new PhabricatorHandleObjectSelectorDataView($handle);
+      $handle_views[$phid] = $view->renderData();
+    }
+    $dialog->addHiddenInput('phids', implode(';', array_keys($this->handles)));
+
+
+    Javelin::initBehavior(
+      'phabricator-object-selector',
+      array(
+        'filter'  => $filter_id,
+        'query'   => $query_id,
+        'search'  => $search_id,
+        'results' => $results_id,
+        'current' => $current_id,
+        'form'    => $form_id,
+        'uri'     => $this->searchURI,
+        'handles' => $handle_views,
+      ));
+
+   return $dialog;
+  }
 
 }

src/view/control/objectselector/__init__.php

@@ -6,7 +6,13 @@
 
 
 
+phutil_require_module('phabricator', 'applications/phid/handle/view/selector');
+phutil_require_module('phabricator', 'infrastructure/celerity/api');
+phutil_require_module('phabricator', 'infrastructure/javelin/api');
+phutil_require_module('phabricator', 'infrastructure/javelin/markup');
 phutil_require_module('phabricator', 'view/dialog');
 
+phutil_require_module('phutil', 'markup');
+
 
 phutil_require_source('PhabricatorObjectSelectorDialog.php');

src/view/dialog/AphrontDialogView.php

@@ -25,6 +25,8 @@ class AphrontDialogView extends AphrontView {
   private $user;
   private $hidden = array();
   private $class;
+  private $renderAsForm = true;
+  private $formID;
 
   public function setUser(PhabricatorUser $user) {
     $this->user = $user;
@@ -56,7 +58,7 @@ class AphrontDialogView extends AphrontView {
   }
 
   public function addHiddenInput($key, $value) {
-    $this->hidden[$key] = $value;
+    $this->hidden[] = array($key, $value);
     return $this;
   }
 
@@ -65,6 +67,17 @@ class AphrontDialogView extends AphrontView {
     return $this;
   }
 
+  public function setRenderDialogAsDiv() {
+    // TODO: This API is awkward.
+    $this->renderAsForm = false;
+    return $this;
+  }
+
+  public function setFormID($id) {
+    $this->formID = $id;
+    return $this;
+  }
+
   final public function render() {
     require_celerity_resource('aphront-dialog-view-css');
 
@@ -90,39 +103,52 @@ class AphrontDialogView extends AphrontView {
         ),
         'Cancel');
     }
+    $buttons = implode('', $buttons);
 
     if (!$this->user) {
       throw new Exception(
         "You must call setUser() when rendering an AphrontDialogView.");
     }
-    $csrf = $this->user->getCSRFToken();
+
+    $more = $this->class;
+
+    $attributes = array(
+      'class'   => 'aphront-dialog-view '.$more,
+      'sigil'   => 'jx-dialog',
+    );
+
+    $form_attributes = array(
+      'action'  => $this->submitURI,
+      'method'  => 'post',
+      'id'      => $this->formID,
+    );
 
     $hidden_inputs = array();
-    foreach ($this->hidden as $key => $value) {
+    foreach ($this->hidden as $desc) {
-      $hidden_inputs[] = phutil_render_tag(
+      list($key, $value) = $desc;
+      $hidden_inputs[] = javelin_render_tag(
         'input',
         array(
           'type' => 'hidden',
           'name' => $key,
           'value' => $value,
+          'sigil' => 'aphront-dialog-application-input'
         ));
     }
     $hidden_inputs = implode("\n", $hidden_inputs);
-
+    $hidden_inputs =
-    $more = $this->class;
-
-    return javelin_render_tag(
-      'form',
-      array(
-        'class'   => 'aphront-dialog-view '.$more,
-        'action'  => $this->submitURI,
-        'method'  => 'post',
-        'sigil'   => 'jx-dialog',
-      ),
-      '<input type="hidden" name="__form__" value="1" />'.
-      '<input type="hidden" name="__csrf__" value="'.$csrf.'" />'.
       '<input type="hidden" name="__dialog__" value="1" />'.
-      $hidden_inputs.
+      $hidden_inputs;
+
+
+    if (!$this->renderAsForm) {
+      $buttons = phabricator_render_form(
+        $this->user,
+        $form_attributes,
+        $hidden_inputs.$buttons);
+    }
+
+    $content =
       '<div class="aphront-dialog-head">'.
         phutil_escape_html($this->title).
       '</div>'.
@@ -130,9 +156,22 @@ class AphrontDialogView extends AphrontView {
         $this->renderChildren().
       '</div>'.
       '<div class="aphront-dialog-tail">'.
-        implode('', $buttons).
+        $buttons.
         '<div style="clear: both;"></div>'.
-      '</div>');
+      '</div>';
+
+    if ($this->renderAsForm) {
+      return phabricator_render_form(
+        $this->user,
+        $form_attributes + $attributes,
+        $hidden_inputs.
+        $content);
+    } else {
+      return javelin_render_tag(
+        'div',
+        $attributes,
+        $content);
+    }
   }
 
 }

webroot/rsrc/css/aphront/dialog-view.css

@@ -58,3 +58,11 @@
   height:     100%;
   min-height: 100%;
 }
+
+.aphront-exception-dialog {
+  width: 95%;
+}
+
+.aphront-exception-dialog .aphront-dialog-head {
+  background: #aa0000;
+}

webroot/rsrc/css/application/objectselector/object-selector.css

@@ -3,11 +3,11 @@
  * @requires aphront-dialog-view-css
  */
 
-form.phabricator-object-selector-dialog {
+.phabricator-object-selector-dialog {
-  width: 800px;
+  width: 960px;
 }
 
-form.phabricator-object-selector-dialog .aphront-dialog-body {
+.phabricator-object-selector-dialog .aphront-dialog-body {
   padding: 0;
 }
 
@@ -30,7 +30,7 @@ td.phabricator-object-selector-search-text {
 
 .phabricator-object-selector-results {
   position: relative;
-  height: 16em;
+  height: 24em;
   border: solid #bbbbbb;
   border-width: 1px 0px;
   overflow-y: scroll;
@@ -79,3 +79,10 @@ td.phabricator-object-selector-search-text {
   background: #ededed;
   padding: 8px 8px;
 }
+
+
+.object-selector-nothing {
+  padding: 1em;
+  color: #888888;
+  text-align: center;
+}

webroot/rsrc/js/application/core/behavior-object-selector.js

@@ -7,9 +7,17 @@ JX.behavior('phabricator-object-selector', function(config) {
   var n = 0;
 
   var phids = {};
-  var handles = {};
+  var handles = config.handles;
+  for (var k in handles) {
+    phids[k] = true;
+  }
   var attach_list = {};
 
+  var phid_input = JX.DOM.find(
+    JX.$(config.form),
+    'input',
+    'aphront-dialog-application-input');
+
   function onreceive(seq, r) {
     if (seq != n) {
       return;
@@ -41,18 +49,26 @@ JX.behavior('phabricator-object-selector', function(config) {
     }
 
     JX.DOM.setContent(JX.$(config.current), display);
+    phid_input.value = JX.keys(phids).join(';');
   }
 
   function renderHandle(h, attach) {
 
+    var link = JX.$N(
+      'a',
+      {href : h.uri, target : '_blank'},
+      h.name);
+
     var td = JX.$N('td');
+
     var table = JX.$N(
       'table',
       {className: 'phabricator-object-selector-handle'},
       JX.$N(
         'tbody',
         {},
-        [JX.$N('th', {}, h.name), td]));
+        [JX.$N('th', {}, link), td]));
+
     var btn = JX.$N(
       'a',
       {className: 'button small grey'},
@@ -62,6 +78,10 @@ JX.behavior('phabricator-object-selector', function(config) {
     JX.Stratcom.addData(btn, {handle : h, table : table});
     if (attach) {
       attach_list[h.phid] = btn;
+      if (h.phid in phids) {
+        JX.DOM.alterClass(btn, 'disabled', true);
+        btn.disabled = true;
+      }
     }
 
     JX.DOM.setContent(td, btn);
@@ -70,7 +90,7 @@ JX.behavior('phabricator-object-selector', function(config) {
   }
 
   function renderNote(note) {
-    return JX.$N('div', {}, note);
+    return JX.$N('div', {className : 'object-selector-nothing'}, note);
   }
 
   function sendQuery() {
@@ -85,7 +105,7 @@ JX.behavior('phabricator-object-selector', function(config) {
 
   JX.DOM.listen(
     JX.$(config.search),
-    'click',
+    'submit',
     null,
     function(e) {
       e.kill();