diff --git a/conf/default.conf.php b/conf/default.conf.php index ccad6cc3bc..f533093c62 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -787,8 +787,6 @@ return array( // -- 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 index 0e58833863..26d32e8053 100644 --- a/src/applications/config/check/PhabricatorSetupCheckExtraConfig.php +++ b/src/applications/config/check/PhabricatorSetupCheckExtraConfig.php @@ -180,6 +180,8 @@ final class PhabricatorSetupCheckExtraConfig extends PhabricatorSetupCheck { '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 index 907f9a3fe6..6e088d32d9 100644 --- a/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php +++ b/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php @@ -13,12 +13,6 @@ final class PhabricatorDifferentialConfigOptions 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', diff --git a/src/applications/differential/controller/DifferentialRevisionDetailRenderer.php b/src/applications/differential/controller/DifferentialRevisionDetailRenderer.php deleted file mode 100644 index 55502c71c0..0000000000 --- a/src/applications/differential/controller/DifferentialRevisionDetailRenderer.php +++ /dev/null @@ -1,46 +0,0 @@ -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 index 23cfa1b906..a4a3046e78 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -41,6 +41,8 @@ final class DifferentialRevisionViewController extends DifferentialController { "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 @@ final class DifferentialRevisionViewController extends DifferentialController { $target_manual->getID()); $props = mpull($props, 'getData', 'getName'); - $aux_fields = $this->loadAuxiliaryFields($revision); - $comments = $revision->loadComments(); $all_changesets = $changesets; @@ -113,25 +113,8 @@ final class DifferentialRevisionViewController extends DifferentialController { } } - $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 @@ final class DifferentialRevisionViewController extends DifferentialController { $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 @@ final class DifferentialRevisionViewController extends DifferentialController { $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 @@ final class DifferentialRevisionViewController extends DifferentialController { $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 @@ final class DifferentialRevisionViewController extends DifferentialController { 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 index a5f044cc64..fd8fed7190 100644 --- a/src/applications/differential/customfield/DifferentialBlameRevisionField.php +++ b/src/applications/differential/customfield/DifferentialBlameRevisionField.php @@ -15,6 +15,10 @@ final class DifferentialBlameRevisionField 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 index 28a5e767da..2c5c65b8c1 100644 --- a/src/applications/differential/customfield/DifferentialHostField.php +++ b/src/applications/differential/customfield/DifferentialHostField.php @@ -15,6 +15,10 @@ final class DifferentialHostField 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 index 969aa61115..1eb0ffbf50 100644 --- a/src/applications/differential/customfield/DifferentialPathField.php +++ b/src/applications/differential/customfield/DifferentialPathField.php @@ -15,6 +15,10 @@ final class DifferentialPathField 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 index 15b1939ba8..2b35612e2b 100644 --- a/src/applications/differential/customfield/DifferentialRevertPlanField.php +++ b/src/applications/differential/customfield/DifferentialRevertPlanField.php @@ -15,6 +15,10 @@ final class DifferentialRevertPlanField 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 index 762a8c73b5..10b4105c3f 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -469,8 +469,7 @@ final class DifferentialRevision extends DifferentialDAO } public function shouldShowSubscribersProperty() { - // TODO: Differential does its own thing for now. - return false; + return true; } public function shouldAllowSubscription($phid) { @@ -483,19 +482,42 @@ final class DifferentialRevision extends DifferentialDAO 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 index d375ede2bd..f261f0ff90 100644 --- a/src/applications/differential/view/DifferentialAddCommentView.php +++ b/src/applications/differential/view/DifferentialAddCommentView.php @@ -6,7 +6,6 @@ final class DifferentialAddCommentView extends AphrontView { private $actions; private $actionURI; private $draft; - private $auxFields; private $reviewers = array(); private $ccs = array(); @@ -15,12 +14,6 @@ final class DifferentialAddCommentView extends AphrontView { 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 @@ final class DifferentialAddCommentView extends AphrontView { )); $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 index 445bdcc177..1294ad9dad 100644 --- a/src/applications/differential/view/DifferentialRevisionDetailView.php +++ b/src/applications/differential/view/DifferentialRevisionDetailView.php @@ -4,7 +4,7 @@ final class DifferentialRevisionDetailView extends AphrontView { private $revision; private $actions; - private $auxiliaryFields = array(); + private $customFields; private $diff; private $uri; @@ -37,9 +37,8 @@ final class DifferentialRevisionDetailView extends AphrontView { 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 @@ final class DifferentialRevisionDetailView extends AphrontView { ->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 @@ final class DifferentialRevisionDetailView extends AphrontView { $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 index fbff6983f9..0000000000 --- a/src/applications/releeph/differential/ReleephDifferentialRevisionDetailRenderer.php +++ /dev/null @@ -1,39 +0,0 @@ -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', - ); - } - -}