Merge branch 'master' into redesign-2015

src/applications/differential/config/PhabricatorDifferentialConfigOptions.php

@@ -34,9 +34,7 @@ final class PhabricatorDifferentialConfigOptions
       new DifferentialReviewedByField(),
       new DifferentialSubscribersField(),
       new DifferentialRepositoryField(),
-      new DifferentialLintField(),
       new DifferentialProjectsField(),
-      new DifferentialUnitField(),
       new DifferentialViewPolicyField(),
       new DifferentialEditPolicyField(),
 
@@ -54,6 +52,8 @@ final class PhabricatorDifferentialConfigOptions
       new DifferentialBlameRevisionField(),
       new DifferentialPathField(),
       new DifferentialHostField(),
+      new DifferentialLintField(),
+      new DifferentialUnitField(),
       new DifferentialRevertPlanField(),
 
       new DifferentialApplyPatchField(),

src/applications/differential/controller/DifferentialRevisionViewController.php

@@ -206,27 +206,6 @@ final class DifferentialRevisionViewController extends DifferentialController {
       $visible_changesets = $changesets;
     }
 
-
-    // TODO: This should be in a DiffQuery or similar.
-    $need_props = array();
-    foreach ($field_list->getFields() as $field) {
-      foreach ($field->getRequiredDiffPropertiesForRevisionView() as $prop) {
-        $need_props[$prop] = $prop;
-      }
-    }
-
-    if ($need_props) {
-      $prop_diff = $revision->getActiveDiff();
-      $load_props = id(new DifferentialDiffProperty())->loadAllWhere(
-        'diffID = %d AND name IN (%Ls)',
-        $prop_diff->getID(),
-        $need_props);
-      $load_props = mpull($load_props, 'getData', 'getName');
-      foreach ($need_props as $need) {
-        $prop_diff->attachProperty($need, idx($load_props, $need));
-      }
-    }
-
     $commit_hashes = mpull($diffs, 'getSourceControlBaseRevision');
     $local_commits = idx($props, 'local:commits', array());
     foreach ($local_commits as $local_commit) {
@@ -286,6 +265,11 @@ final class DifferentialRevisionViewController extends DifferentialController {
       $revision_detail_box->setInfoView($revision_warnings);
     }
 
+    $diff_detail_box = $this->buildDiffDetailView(
+      array_select_keys($diffs, array($diff_vs, $target->getID())),
+      $revision,
+      $field_list);
+
     $comment_view = $this->buildTransactions(
       $revision,
       $diff_vs ? $diffs[$diff_vs] : $target,
@@ -486,6 +470,7 @@ final class DifferentialRevisionViewController extends DifferentialController {
 
     $content = array(
       $revision_detail_box,
+      $diff_detail_box,
       $page_pane,
     );
 
@@ -972,4 +957,85 @@ final class DifferentialRevisionViewController extends DifferentialController {
     return $warnings;
   }
 
+  private function buildDiffDetailView(
+    array $diffs,
+    DifferentialRevision $revision,
+    PhabricatorCustomFieldList $field_list) {
+    $viewer = $this->getViewer();
+
+    $fields = array();
+    foreach ($field_list->getFields() as $field) {
+      if ($field->shouldAppearInDiffPropertyView()) {
+        $fields[] = $field;
+      }
+    }
+
+    if (!$fields) {
+      return null;
+    }
+
+    // Make sure we're only going to render unique diffs.
+    $diffs = mpull($diffs, null, 'getID');
+    $labels = array(pht('Left'), pht('Right'));
+
+    $property_lists = array();
+    foreach ($diffs as $diff) {
+      if (count($diffs) == 2) {
+        $label = array_shift($labels);
+        $label = pht('%s (Diff %d)', $label, $diff->getID());
+      } else {
+        $label = pht('Diff %d', $diff->getID());
+      }
+
+      $property_lists[] = array(
+        $label,
+        $this->buildDiffPropertyList($diff, $revision, $fields),
+      );
+    }
+
+    $box = id(new PHUIObjectBoxView())
+      ->setHeaderText(pht('Diff Detail'))
+      ->setUser($viewer);
+
+    $last_tab = null;
+    foreach ($property_lists as $key => $property_list) {
+      list($tab_name, $list_view) = $property_list;
+
+      $tab = id(new PHUIListItemView())
+        ->setKey($key)
+        ->setName($tab_name);
+
+      $box->addPropertyList($list_view, $tab);
+      $last_tab = $tab;
+    }
+
+    if ($last_tab) {
+      $last_tab->setSelected(true);
+    }
+
+    return $box;
+  }
+
+  private function buildDiffPropertyList(
+    DifferentialDiff $diff,
+    DifferentialRevision $revision,
+    array $fields) {
+    $viewer = $this->getViewer();
+
+    $view = id(new PHUIPropertyListView())
+      ->setUser($viewer)
+      ->setObject($diff);
+
+    foreach ($fields as $field) {
+      $label = $field->renderDiffPropertyViewLabel($diff);
+      $value = $field->renderDiffPropertyViewValue($diff);
+      if ($value !== null) {
+        $view->addProperty($label, $value);
+      }
+    }
+
+    return $view;
+  }
+
+
 }

src/applications/differential/customfield/DifferentialBranchField.php

@@ -19,12 +19,20 @@ final class DifferentialBranchField
     return true;
   }
 
-  public function renderPropertyViewLabel() {
+  public function renderPropertyViewValue(array $handles) {
+    return null;
+  }
+
+  public function shouldAppearInDiffPropertyView() {
+    return true;
+  }
+
+  public function renderDiffPropertyViewLabel(DifferentialDiff $diff) {
     return $this->getFieldName();
   }
 
-  public function renderPropertyViewValue(array $handles) {
-    return $this->getBranchDescription($this->getObject()->getActiveDiff());
+  public function renderDiffPropertyViewValue(DifferentialDiff $diff) {
+    return $this->getBranchDescription($diff);
   }
 
   private function getBranchDescription(DifferentialDiff $diff) {

src/applications/differential/customfield/DifferentialCustomField.php

@@ -2,6 +2,7 @@
 
 /**
  * @task commitmessage    Integration with Commit Messages
+ * @task diff             Integration with Diff Properties
  */
 abstract class DifferentialCustomField
   extends PhabricatorCustomField {
@@ -31,13 +32,6 @@ abstract class DifferentialCustomField
     return parent::shouldEnableForRole($role);
   }
 
-  public function getRequiredDiffPropertiesForRevisionView() {
-    if ($this->getProxy()) {
-      return $this->getProxy()->getRequiredDiffPropertiesForRevisionView();
-    }
-    return array();
-  }
-
   protected function parseObjectList(
     $value,
     array $types,
@@ -82,6 +76,7 @@ abstract class DifferentialCustomField
     return array();
   }
 
+
 /* -(  Integration with Commit Messages  )----------------------------------- */
 
 
@@ -217,4 +212,40 @@ abstract class DifferentialCustomField
     return;
   }
 
+
+/* -(  Integration with Diff Properties  )----------------------------------- */
+
+
+  /**
+   * @task diff
+   */
+  public function shouldAppearInDiffPropertyView() {
+    if ($this->getProxy()) {
+      return $this->getProxy()->shouldAppearInDiffPropertyView();
+    }
+    return false;
+  }
+
+
+  /**
+   * @task diff
+   */
+  public function renderDiffPropertyViewLabel(DifferentialDiff $diff) {
+    if ($this->proxy) {
+      return $this->proxy->renderDiffPropertyViewLabel($diff);
+    }
+    return $this->getFieldName();
+  }
+
+
+  /**
+   * @task diff
+   */
+  public function renderDiffPropertyViewValue(DifferentialDiff $diff) {
+    if ($this->proxy) {
+      return $this->proxy->renderDiffPropertyViewValue($diff);
+    }
+    throw new PhabricatorCustomFieldImplementationIncompleteException($this);
+  }
+
 }

src/applications/differential/customfield/DifferentialHostField.php

@@ -23,12 +23,20 @@ final class DifferentialHostField
     return true;
   }
 
-  public function renderPropertyViewLabel() {
+  public function renderPropertyViewValue(array $handles) {
+    return null;
+  }
+
+  public function shouldAppearInDiffPropertyView() {
+    return true;
+  }
+
+  public function renderDiffPropertyViewLabel(DifferentialDiff $diff) {
     return $this->getFieldName();
   }
 
-  public function renderPropertyViewValue(array $handles) {
-    $host = $this->getObject()->getActiveDiff()->getSourceMachine();
+  public function renderDiffPropertyViewValue(DifferentialDiff $diff) {
+    $host = $diff->getSourceMachine();
     if (!$host) {
       return null;
     }

src/applications/differential/customfield/DifferentialLintField.php

@@ -19,20 +19,37 @@ final class DifferentialLintField
     return true;
   }
 
-  public function renderPropertyViewLabel() {
+  public function renderPropertyViewValue(array $handles) {
+    return null;
+  }
+
+  public function shouldAppearInDiffPropertyView() {
+    return true;
+  }
+
+  public function renderDiffPropertyViewLabel(DifferentialDiff $diff) {
     return $this->getFieldName();
   }
 
-  public function getRequiredDiffPropertiesForRevisionView() {
-    return array(
+  public function renderDiffPropertyViewValue(DifferentialDiff $diff) {
+    // TODO: This load is slightly inefficient, but most of this is moving
+    // to Harbormaster and this simplifies the transition. Eat 1-2 extra
+    // queries for now.
+    $keys = array(
       'arc:lint',
       'arc:lint-excuse',
       'arc:lint-postponed',
     );
-  }
 
-  public function renderPropertyViewValue(array $handles) {
-    $diff = $this->getObject()->getActiveDiff();
+    $properties = id(new DifferentialDiffProperty())->loadAllWhere(
+      'diffID = %d AND name IN (%Ls)',
+      $diff->getID(),
+      $keys);
+    $properties = mpull($properties, 'getData', 'getName');
+
+    foreach ($keys as $key) {
+      $diff->attachProperty($key, idx($properties, $key));
+    }
 
     $path_changesets = mpull($diff->loadChangesets(), 'getID', 'getFilename');
 

src/applications/differential/customfield/DifferentialPathField.php

@@ -23,12 +23,20 @@ final class DifferentialPathField
     return true;
   }
 
-  public function renderPropertyViewLabel() {
+  public function renderPropertyViewValue(array $handles) {
+    return null;
+  }
+
+  public function shouldAppearInDiffPropertyView() {
+    return true;
+  }
+
+  public function renderDiffPropertyViewLabel(DifferentialDiff $diff) {
     return $this->getFieldName();
   }
 
-  public function renderPropertyViewValue(array $handles) {
-    $path = $this->getObject()->getActiveDiff()->getSourcePath();
+  public function renderDiffPropertyViewValue(DifferentialDiff $diff) {
+    $path = $diff->getSourcePath();
     if (!$path) {
       return null;
     }

src/applications/differential/customfield/DifferentialRepositoryField.php

@@ -125,20 +125,24 @@ final class DifferentialRepositoryField
     return true;
   }
 
-  public function renderPropertyViewLabel() {
+  public function renderPropertyViewValue(array $handles) {
+    return null;
+  }
+
+  public function shouldAppearInDiffPropertyView() {
+    return true;
+  }
+
+  public function renderDiffPropertyViewLabel(DifferentialDiff $diff) {
     return $this->getFieldName();
   }
 
-  public function getRequiredHandlePHIDsForPropertyView() {
-    $repository_phid = $this->getObject()->getRepositoryPHID();
-    if ($repository_phid) {
-      return array($repository_phid);
-    }
-    return array();
+  public function renderDiffPropertyViewValue(DifferentialDiff $diff) {
+    if (!$diff->getRepositoryPHID()) {
+      return null;
     }
 
-  public function renderPropertyViewValue(array $handles) {
-    return $this->renderHandleList($handles);
+    return $this->getViewer()->renderHandle($diff->getRepositoryPHID());
   }
 
   public function shouldAppearInTransactionMail() {

src/applications/differential/customfield/DifferentialUnitField.php

@@ -19,19 +19,34 @@ final class DifferentialUnitField
     return true;
   }
 
-  public function renderPropertyViewLabel() {
+  public function renderPropertyViewValue(array $handles) {
+    return null;
+  }
+
+  public function shouldAppearInDiffPropertyView() {
+    return true;
+  }
+
+  public function renderDiffPropertyViewLabel(DifferentialDiff $diff) {
     return $this->getFieldName();
   }
 
-  public function getRequiredDiffPropertiesForRevisionView() {
-    return array(
+  public function renderDiffPropertyViewValue(DifferentialDiff $diff) {
+    // TODO: See DifferentialLintField.
+    $keys = array(
       'arc:unit',
       'arc:unit-excuse',
     );
-  }
 
-  public function renderPropertyViewValue(array $handles) {
-    $diff = $this->getObject()->getActiveDiff();
+    $properties = id(new DifferentialDiffProperty())->loadAllWhere(
+      'diffID = %d AND name IN (%Ls)',
+      $diff->getID(),
+      $keys);
+    $properties = mpull($properties, 'getData', 'getName');
+
+    foreach ($keys as $key) {
+      $diff->attachProperty($key, idx($properties, $key));
+    }
 
     $ustar = DifferentialRevisionUpdateHistoryView::renderDiffUnitStar($diff);
     $umsg = DifferentialRevisionUpdateHistoryView::getDiffUnitMessage($diff);

src/applications/harbormaster/event/HarbormasterUIEventListener.php

@@ -31,6 +31,13 @@ final class HarbormasterUIEventListener
       return;
     }
 
+    if ($object instanceof DifferentialRevision) {
+      // TODO: This is a bit hacky and we could probably find a cleaner fix
+      // eventually, but we show build status on each diff, immediately below
+      // this property list, so it's redundant to show it on the revision view.
+      return;
+    }
+
     if (!($object instanceof HarbormasterBuildableInterface)) {
       return;
     }

src/applications/policy/controller/PhabricatorPolicyExplainController.php

@@ -69,7 +69,27 @@ final class PhabricatorPolicyExplainController
       $capability_name = $capobj->getCapabilityName();
     }
 
+    $space_info = null;
+    if ($object instanceof PhabricatorSpacesInterface) {
+      if (PhabricatorSpacesNamespaceQuery::getViewerSpacesExist($viewer)) {
+        $space_phid = PhabricatorSpacesNamespaceQuery::getObjectSpacePHID(
+          $object);
+
+        $handles = $viewer->loadHandles(array($space_phid));
+
+        $space_info = array(
+          pht(
+            'This object is in %s, and can only be seen by users with '.
+            'access to that space.',
+            $handles[$space_phid]->renderLink()),
+          phutil_tag('br'),
+          phutil_tag('br'),
+        );
+      }
+    }
+
     $content = array(
+      $space_info,
       pht('Users with the "%s" capability:', $capability_name),
       $auto_info,
     );