Page MenuHomePhabricator
Files F13991873

D8361.id19875.diff
No OneTemporary
Actions

D8361.id19875.diff
View Options

diff --git a/conf/default.conf.php b/conf/default.conf.php
--- a/conf/default.conf.php
+++ b/conf/default.conf.php
@@ -787,8 +787,6 @@
// -- Differential ---------------------------------------------------------- //
- 'differential.revision-custom-detail-renderer' => null,
-
// List of file regexps where whitespace is meaningful and should not
// use 'ignore-all' by default
'differential.whitespace-matters' => array(
diff --git a/src/applications/config/check/PhabricatorSetupCheckExtraConfig.php b/src/applications/config/check/PhabricatorSetupCheckExtraConfig.php
--- a/src/applications/config/check/PhabricatorSetupCheckExtraConfig.php
+++ b/src/applications/config/check/PhabricatorSetupCheckExtraConfig.php
@@ -180,6 +180,8 @@
'auth.sessions.web' => $session_reason,
'tokenizer.ondemand' => pht(
'Phabricator now manages typeahead strategies automatically.'),
+ 'differential.revision-custom-detail-renderer' => pht(
+ 'Obsolete; use standard rendering events instead.'),
);
return $ancient_config;
diff --git a/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php b/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php
--- a/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php
+++ b/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php
@@ -14,12 +14,6 @@
public function getOptions() {
return array(
$this->newOption(
- 'differential.revision-custom-detail-renderer',
- 'class',
- null)
- ->setBaseClass('DifferentialRevisionDetailRenderer')
- ->setDescription(pht("Custom revision detail renderer.")),
- $this->newOption(
'differential.whitespace-matters',
'list<regex>',
array(
diff --git a/src/applications/differential/controller/DifferentialRevisionDetailRenderer.php b/src/applications/differential/controller/DifferentialRevisionDetailRenderer.php
deleted file mode 100644
--- a/src/applications/differential/controller/DifferentialRevisionDetailRenderer.php
+++ /dev/null
@@ -1,46 +0,0 @@
-<?php
-
-abstract class DifferentialRevisionDetailRenderer {
- private $user;
- private $diff;
- private $vsDiff;
-
- final public function setUser(PhabricatorUser $user) {
- $this->user = $user;
- return $this;
- }
-
- final protected function getUser() {
- return $this->user;
- }
-
- final public function setDiff(DifferentialDiff $diff) {
- $this->diff = $diff;
- return $this;
- }
-
- final protected function getDiff() {
- return $this->diff;
- }
-
- final public function setVSDiff(DifferentialDiff $diff) {
- $this->vsDiff = $diff;
- return $this;
- }
-
- final protected function getVSDiff() {
- return $this->vsDiff;
- }
-
- /**
- * This function must return an array of action links that will be
- * added to the end of action links on the differential revision
- * page. Each element in the array must be an array which must
- * contain 'name' and 'href' fields. 'name' will be the name of the
- * link and 'href' will be the address where the link points
- * to. 'class' is optional and can be used for specifying a CSS
- * class.
- */
- abstract public function generateActionLinks(DifferentialRevision $revision,
- DifferentialDiff $diff);
-}
diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -41,6 +41,8 @@
"This revision has no diffs. Something has gone quite wrong.");
}
+ $revision->attachActiveDiff(last($diffs));
+
$diff_vs = $request->getInt('vs');
$target_id = $request->getInt('id');
@@ -82,8 +84,6 @@
$target_manual->getID());
$props = mpull($props, 'getData', 'getName');
- $aux_fields = $this->loadAuxiliaryFields($revision);
-
$comments = $revision->loadComments();
$all_changesets = $changesets;
@@ -113,25 +113,8 @@
}
}
- $aux_phids = array();
- foreach ($aux_fields as $key => $aux_field) {
- $aux_field->setDiff($target);
- $aux_field->setManualDiff($target_manual);
- $aux_field->setDiffProperties($props);
- $aux_phids[$key] = $aux_field->getRequiredHandlePHIDsForRevisionView();
- }
- $object_phids = array_merge($object_phids, array_mergev($aux_phids));
- $object_phids = array_unique($object_phids);
-
$handles = $this->loadViewerHandles($object_phids);
- foreach ($aux_fields as $key => $aux_field) {
- // Make sure each field only has access to handles it specifically
- // requested, not all handles. Otherwise you can get a field which works
- // only in the presence of other fields.
- $aux_field->setHandles(array_select_keys($handles, $aux_phids[$key]));
- }
-
$request_uri = $request->getRequestURI();
$limit = 100;
@@ -184,32 +167,22 @@
$visible_changesets = $changesets;
}
+ $field_list = PhabricatorCustomField::getObjectFields(
+ $revision,
+ PhabricatorCustomField::ROLE_VIEW);
+
+ $field_list->setViewer($user);
+ $field_list->readFieldsFromStorage($revision);
+
$revision_detail = id(new DifferentialRevisionDetailView())
->setUser($user)
->setRevision($revision)
->setDiff(end($diffs))
- ->setAuxiliaryFields($aux_fields)
+ ->setCustomFields($field_list)
->setURI($request->getRequestURI());
$actions = $this->getRevisionActions($revision);
- $custom_renderer_class = PhabricatorEnv::getEnvConfig(
- 'differential.revision-custom-detail-renderer');
- if ($custom_renderer_class) {
-
- // TODO: build a better version of the action links and deprecate the
- // whole DifferentialRevisionDetailRenderer class.
- $custom_renderer = newv($custom_renderer_class, array());
- $custom_renderer->setUser($user);
- $custom_renderer->setDiff($target);
- if ($diff_vs) {
- $custom_renderer->setVSDiff($diffs[$diff_vs]);
- }
- $actions = array_merge(
- $actions,
- $custom_renderer->generateActionLinks($revision, $target_manual));
- }
-
$whitespace = $request->getStr(
'whitespace',
DifferentialChangesetParser::WHITESPACE_IGNORE_ALL);
@@ -340,7 +313,9 @@
$comment_form = new DifferentialAddCommentView();
$comment_form->setRevision($revision);
- $comment_form->setAuxFields($aux_fields);
+
+ // TODO: Restore the ability for fields to add accept warnings.
+
$comment_form->setActions($this->getRevisionCommentActions($revision));
$action_uri = '/differential/comment/save/';
@@ -450,46 +425,42 @@
$revision,
PhabricatorPolicyCapability::CAN_EDIT);
- $links = array();
+ $actions = array();
- $links[] = array(
- 'icon' => 'edit',
- 'href' => "/differential/revision/edit/{$revision_id}/",
- 'name' => pht('Edit Revision'),
- 'disabled' => !$can_edit,
- 'sigil' => $can_edit ? null : 'workflow',
- );
+ $actions[] = id(new PhabricatorActionView())
+ ->setIcon('edit')
+ ->setHref("/differential/revision/edit/{$revision_id}/")
+ ->setName(pht('Edit Revision'))
+ ->setDisabled(!$can_edit)
+ ->setWorkflow(!$can_edit);
$this->requireResource('phabricator-object-selector-css');
$this->requireResource('javelin-behavior-phabricator-object-selector');
- $links[] = array(
- 'icon' => 'link',
- 'name' => pht('Edit Dependencies'),
- 'href' => "/search/attach/{$revision_phid}/DREV/dependencies/",
- 'sigil' => 'workflow',
- 'disabled' => !$can_edit,
- );
+ $actions[] = id(new PhabricatorActionView())
+ ->setIcon('link')
+ ->setName(pht('Edit Dependencies'))
+ ->setHref("/search/attach/{$revision_phid}/DREV/dependencies/")
+ ->setWorkflow(true)
+ ->setDisabled(!$can_edit);
$maniphest = 'PhabricatorApplicationManiphest';
if (PhabricatorApplication::isClassInstalled($maniphest)) {
- $links[] = array(
- 'icon' => 'attach',
- 'name' => pht('Edit Maniphest Tasks'),
- 'href' => "/search/attach/{$revision_phid}/TASK/",
- 'sigil' => 'workflow',
- 'disabled' => !$can_edit,
- );
+ $actions[] = id(new PhabricatorActionView())
+ ->setIcon('attach')
+ ->setName(pht('Edit Maniphest Tasks'))
+ ->setHref("/search/attach/{$revision_phid}/TASK/")
+ ->setWorkflow(true)
+ ->setDisabled(!$can_edit);
}
$request_uri = $this->getRequest()->getRequestURI();
- $links[] = array(
- 'icon' => 'download',
- 'name' => pht('Download Raw Diff'),
- 'href' => $request_uri->alter('download', 'true')
- );
+ $actions[] = id(new PhabricatorActionView())
+ ->setIcon('download')
+ ->setName(pht('Download Raw Diff'))
+ ->setHref($request_uri->alter('download', 'true'));
- return $links;
+ return $actions;
}
private function getRevisionCommentActions(DifferentialRevision $revision) {
@@ -689,25 +660,6 @@
return array($changesets, $vs_map, $vs_changesets, $refs);
}
- private function loadAuxiliaryFields(DifferentialRevision $revision) {
-
- $aux_fields = DifferentialFieldSelector::newSelector()
- ->getFieldSpecifications();
- foreach ($aux_fields as $key => $aux_field) {
- if (!$aux_field->shouldAppearOnRevisionView()) {
- unset($aux_fields[$key]);
- } else {
- $aux_field->setUser($this->getRequest()->getUser());
- }
- }
-
- $aux_fields = DifferentialAuxiliaryField::loadFromStorage(
- $revision,
- $aux_fields);
-
- return $aux_fields;
- }
-
private function buildSymbolIndexes(
PhabricatorRepositoryArcanistProject $arc_project,
array $visible_changesets) {
diff --git a/src/applications/differential/customfield/DifferentialBlameRevisionField.php b/src/applications/differential/customfield/DifferentialBlameRevisionField.php
--- a/src/applications/differential/customfield/DifferentialBlameRevisionField.php
+++ b/src/applications/differential/customfield/DifferentialBlameRevisionField.php
@@ -15,6 +15,10 @@
return pht('Stores a reference to what this fixes.');
}
+ public function shouldDisableByDefault() {
+ return true;
+ }
+
public function shouldAppearInPropertyView() {
return true;
}
diff --git a/src/applications/differential/customfield/DifferentialHostField.php b/src/applications/differential/customfield/DifferentialHostField.php
--- a/src/applications/differential/customfield/DifferentialHostField.php
+++ b/src/applications/differential/customfield/DifferentialHostField.php
@@ -15,6 +15,10 @@
return pht('Shows the local host where the diff came from.');
}
+ public function shouldDisableByDefault() {
+ return true;
+ }
+
public function shouldAppearInPropertyView() {
return true;
}
diff --git a/src/applications/differential/customfield/DifferentialPathField.php b/src/applications/differential/customfield/DifferentialPathField.php
--- a/src/applications/differential/customfield/DifferentialPathField.php
+++ b/src/applications/differential/customfield/DifferentialPathField.php
@@ -15,6 +15,10 @@
return pht('Shows the local path where the diff came from.');
}
+ public function shouldDisableByDefault() {
+ return true;
+ }
+
public function shouldAppearInPropertyView() {
return true;
}
diff --git a/src/applications/differential/customfield/DifferentialRevertPlanField.php b/src/applications/differential/customfield/DifferentialRevertPlanField.php
--- a/src/applications/differential/customfield/DifferentialRevertPlanField.php
+++ b/src/applications/differential/customfield/DifferentialRevertPlanField.php
@@ -15,6 +15,10 @@
return pht('Instructions for reverting/undoing this change.');
}
+ public function shouldDisableByDefault() {
+ return true;
+ }
+
public function shouldAppearInPropertyView() {
return true;
}
diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php
--- a/src/applications/differential/storage/DifferentialRevision.php
+++ b/src/applications/differential/storage/DifferentialRevision.php
@@ -469,8 +469,7 @@
}
public function shouldShowSubscribersProperty() {
- // TODO: Differential does its own thing for now.
- return false;
+ return true;
}
public function shouldAllowSubscription($phid) {
@@ -483,19 +482,42 @@
public function getCustomFieldSpecificationForRole($role) {
$fields = array(
+ new DifferentialAuthorField(),
+
new DifferentialTitleField(),
new DifferentialSummaryField(),
new DifferentialTestPlanField(),
new DifferentialReviewersField(),
+ new DifferentialProjectReviewersField(),
new DifferentialSubscribersField(),
new DifferentialRepositoryField(),
new DifferentialViewPolicyField(),
new DifferentialEditPolicyField(),
+
+ new DifferentialDependsOnField(),
+ new DifferentialDependenciesField(),
+ new DifferentialManiphestTasksField(),
+ new DifferentialCommitsField(),
+
+ new DifferentialJIRAIssuesField(),
+ new DifferentialAsanaRepresentationField(),
+
+ new DifferentialBlameRevisionField(),
+ new DifferentialPathField(),
+ new DifferentialHostField(),
+ new DifferentialRevertPlanField(),
+
+ new DifferentialApplyPatchField(),
);
- return array_fill_keys(
- mpull($fields, 'getFieldKey'),
- array('disabled' => false));
+ $result = array();
+ foreach ($fields as $field) {
+ $result[$field->getFieldKey()] = array(
+ 'disabled' => $field->shouldDisableByDefault(),
+ );
+ }
+
+ return $result;
}
public function getCustomFieldBaseClass() {
diff --git a/src/applications/differential/view/DifferentialAddCommentView.php b/src/applications/differential/view/DifferentialAddCommentView.php
--- a/src/applications/differential/view/DifferentialAddCommentView.php
+++ b/src/applications/differential/view/DifferentialAddCommentView.php
@@ -6,7 +6,6 @@
private $actions;
private $actionURI;
private $draft;
- private $auxFields;
private $reviewers = array();
private $ccs = array();
@@ -15,12 +14,6 @@
return $this;
}
- public function setAuxFields(array $aux_fields) {
- assert_instances_of($aux_fields, 'DifferentialFieldSpecification');
- $this->auxFields = $aux_fields;
- return $this;
- }
-
public function setActions(array $actions) {
$this->actions = $actions;
return $this;
@@ -136,7 +129,7 @@
));
$diff = $revision->loadActiveDiff();
- $warnings = mpull($this->auxFields, 'renderWarningBoxForRevisionAccept');
+ $warnings = array();
Javelin::initBehavior(
'differential-accept-with-errors',
diff --git a/src/applications/differential/view/DifferentialRevisionDetailView.php b/src/applications/differential/view/DifferentialRevisionDetailView.php
--- a/src/applications/differential/view/DifferentialRevisionDetailView.php
+++ b/src/applications/differential/view/DifferentialRevisionDetailView.php
@@ -4,7 +4,7 @@
private $revision;
private $actions;
- private $auxiliaryFields = array();
+ private $customFields;
private $diff;
private $uri;
@@ -37,9 +37,8 @@
return $this->actions;
}
- public function setAuxiliaryFields(array $fields) {
- assert_instances_of($fields, 'DifferentialFieldSpecification');
- $this->auxiliaryFields = $fields;
+ public function setCustomFields(PhabricatorCustomFieldList $list) {
+ $this->customFields = $list;
return $this;
}
@@ -57,15 +56,7 @@
->setObject($revision)
->setObjectURI($this->getURI());
foreach ($this->getActions() as $action) {
- $obj = id(new PhabricatorActionView())
- ->setIcon(idx($action, 'icon', 'edit'))
- ->setName($action['name'])
- ->setHref(idx($action, 'href'))
- ->setWorkflow(idx($action, 'sigil') == 'workflow')
- ->setRenderAsForm(!empty($action['instant']))
- ->setUser($user)
- ->setDisabled(idx($action, 'disabled', false));
- $actions->addAction($obj);
+ $actions->addAction($action);
}
$properties = id(new PHUIPropertyListView())
@@ -102,42 +93,15 @@
$properties->addProperty(pht('Next Step'), $next_step);
}
- foreach ($this->auxiliaryFields as $field) {
- $value = $field->renderValueForRevisionView();
- if ($value !== null) {
- $label = rtrim($field->renderLabelForRevisionView(), ':');
- $properties->addProperty($label, $value);
- }
- }
$properties->setHasKeyboardShortcuts(true);
$properties->setActionList($actions);
- $properties->invokeWillRenderEvent();
-
- if (strlen($revision->getSummary())) {
- $properties->addSectionHeader(
- pht('Summary'),
- PHUIPropertyListView::ICON_SUMMARY);
- $properties->addTextContent(
- PhabricatorMarkupEngine::renderOneObject(
- id(new PhabricatorMarkupOneOff())
- ->setPreserveLinebreaks(true)
- ->setContent($revision->getSummary()),
- 'default',
- $user));
- }
-
- if (strlen($revision->getTestPlan())) {
- $properties->addSectionHeader(
- pht('Test Plan'),
- PHUIPropertyListView::ICON_TESTPLAN);
- $properties->addTextContent(
- PhabricatorMarkupEngine::renderOneObject(
- id(new PhabricatorMarkupOneOff())
- ->setPreserveLinebreaks(true)
- ->setContent($revision->getTestPlan()),
- 'default',
- $user));
+ $field_list = $this->customFields;
+ if ($field_list) {
+ $field_list->appendFieldsToPropertyList(
+ $revision,
+ $user,
+ $properties);
}
$object_box = id(new PHUIObjectBoxView())
diff --git a/src/applications/releeph/differential/ReleephDifferentialRevisionDetailRenderer.php b/src/applications/releeph/differential/ReleephDifferentialRevisionDetailRenderer.php
deleted file mode 100644
--- a/src/applications/releeph/differential/ReleephDifferentialRevisionDetailRenderer.php
+++ /dev/null
@@ -1,39 +0,0 @@
-<?php
-
-final class ReleephDifferentialRevisionDetailRenderer {
-
- public static function generateActionLink(DifferentialRevision $revision,
- DifferentialDiff $diff) {
-
- $arc_project = $diff->loadArcanistProject(); // 93us
- if (!$arc_project) {
- return;
- }
-
- $releeph_projects = id(new ReleephProject())->loadAllWhere(
- 'arcanistProjectID = %d AND isActive = 1',
- $arc_project->getID());
-
- if (!$releeph_projects) {
- return;
- }
-
- $releeph_branches = id(new ReleephBranch())->loadAllWhere(
- 'releephProjectID IN (%Ld) AND isActive = 1',
- mpull($releeph_projects, 'getID'));
-
- if (!$releeph_branches) {
- return;
- }
-
- $uri = new PhutilURI(
- '/releeph/request/differentialcreate/D'.$revision->getID());
- return array(
- 'name' => 'Releeph Request',
- 'sigil' => 'workflow',
- 'href' => $uri,
- 'icon' => 'fork',
- );
- }
-
-}

File Metadata

Mime Type
text/plain
Expires
Wed, Oct 23, 1:12 PM (1 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6744264
Default Alt Text
D8361.id19875.diff (19 KB)

Event Timeline

Phabricator · Built by Phacility