1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-19 12:00:55 +01:00

Move Differential's simple fields to the extensible field schema

Summary:
Differential has a bunch of display-only fields, implement them all as field
specifications instead of hard-coded fields.

Also add some more documentation and fix redundant string constants in blame
rev/revert plan fields.

Test Plan: Viewed, edited, and hit conduit for revisions.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, jungejason, epriestley
Differential Revision: 807
This commit is contained in:
epriestley 2011-08-14 11:29:56 -07:00
parent e5ecd784ec
commit 52ec6c02ee
23 changed files with 465 additions and 40 deletions

View file

@ -133,6 +133,7 @@ phutil_register_library_map(array(
'DatabaseConfigurationProvider' => 'applications/base/storage/configuration',
'DifferentialAction' => 'applications/differential/constants/action',
'DifferentialAddCommentView' => 'applications/differential/view/addcomment',
'DifferentialApplyPatchFieldSpecification' => 'applications/differential/field/specification/applypatch',
'DifferentialAuxiliaryField' => 'applications/differential/storage/auxiliaryfield',
'DifferentialBlameRevisionFieldSpecification' => 'applications/differential/field/specification/blamerev',
'DifferentialCCWelcomeMail' => 'applications/differential/mail/ccwelcome',
@ -162,18 +163,23 @@ phutil_register_library_map(array(
'DifferentialDiffTableOfContentsView' => 'applications/differential/view/difftableofcontents',
'DifferentialDiffViewController' => 'applications/differential/controller/diffview',
'DifferentialExceptionMail' => 'applications/differential/mail/exception',
'DifferentialExportPatchFieldSpecification' => 'applications/differential/field/specification/exportpatch',
'DifferentialFieldDataNotAvailableException' => 'applications/differential/field/exception/notavailable',
'DifferentialFieldSelector' => 'applications/differential/field/selector/base',
'DifferentialFieldSpecification' => 'applications/differential/field/specification/base',
'DifferentialFieldSpecificationIncompleteException' => 'applications/differential/field/exception/incomplete',
'DifferentialFieldValidationException' => 'applications/differential/field/exception/validation',
'DifferentialHostFieldSpecification' => 'applications/differential/field/specification/host',
'DifferentialHunk' => 'applications/differential/storage/hunk',
'DifferentialInlineComment' => 'applications/differential/storage/inlinecomment',
'DifferentialInlineCommentEditController' => 'applications/differential/controller/inlinecommentedit',
'DifferentialInlineCommentPreviewController' => 'applications/differential/controller/inlinecommentpreview',
'DifferentialInlineCommentView' => 'applications/differential/view/inlinecomment',
'DifferentialLinesFieldSpecification' => 'applications/differential/field/specification/lines',
'DifferentialLintStatus' => 'applications/differential/constants/lintstatus',
'DifferentialMail' => 'applications/differential/mail/base',
'DifferentialNewDiffMail' => 'applications/differential/mail/newdiff',
'DifferentialPathFieldSpecification' => 'applications/differential/field/specification/path',
'DifferentialPrimaryPaneView' => 'applications/differential/view/primarypane',
'DifferentialReplyHandler' => 'applications/differential/replyhandler',
'DifferentialRevertPlanFieldSpecification' => 'applications/differential/field/specification/revertplan',
@ -775,6 +781,7 @@ phutil_register_library_map(array(
'DarkConsoleServicesPlugin' => 'DarkConsolePlugin',
'DarkConsoleXHProfPlugin' => 'DarkConsolePlugin',
'DifferentialAddCommentView' => 'AphrontView',
'DifferentialApplyPatchFieldSpecification' => 'DifferentialFieldSpecification',
'DifferentialAuxiliaryField' => 'DifferentialDAO',
'DifferentialBlameRevisionFieldSpecification' => 'DifferentialFieldSpecification',
'DifferentialCCWelcomeMail' => 'DifferentialReviewRequestMail',
@ -796,12 +803,16 @@ phutil_register_library_map(array(
'DifferentialDiffTableOfContentsView' => 'AphrontView',
'DifferentialDiffViewController' => 'DifferentialController',
'DifferentialExceptionMail' => 'DifferentialMail',
'DifferentialExportPatchFieldSpecification' => 'DifferentialFieldSpecification',
'DifferentialHostFieldSpecification' => 'DifferentialFieldSpecification',
'DifferentialHunk' => 'DifferentialDAO',
'DifferentialInlineComment' => 'DifferentialDAO',
'DifferentialInlineCommentEditController' => 'DifferentialController',
'DifferentialInlineCommentPreviewController' => 'DifferentialController',
'DifferentialInlineCommentView' => 'AphrontView',
'DifferentialLinesFieldSpecification' => 'DifferentialFieldSpecification',
'DifferentialNewDiffMail' => 'DifferentialReviewRequestMail',
'DifferentialPathFieldSpecification' => 'DifferentialFieldSpecification',
'DifferentialPrimaryPaneView' => 'AphrontView',
'DifferentialReplyHandler' => 'PhabricatorMailReplyHandler',
'DifferentialRevertPlanFieldSpecification' => 'DifferentialFieldSpecification',

View file

@ -116,7 +116,7 @@ class ConduitAPI_differential_getrevision_Method extends ConduitAPIMethod {
$revision,
$aux_fields);
return mpull($aux_fields, 'getValueForConduit', 'getStorageKey');
return mpull($aux_fields, 'getValueForConduit', 'getKeyForConduit');
}
}

View file

@ -43,8 +43,6 @@ class DifferentialRevisionViewController extends DifferentialController {
"This revision has no diffs. Something has gone quite wrong.");
}
$aux_fields = $this->loadAuxiliaryFields($revision);
$diff_vs = $request->getInt('vs');
$target = end($diffs);
@ -60,6 +58,11 @@ class DifferentialRevisionViewController extends DifferentialController {
$diff_vs = null;
}
$aux_fields = $this->loadAuxiliaryFields($revision);
foreach ($aux_fields as $aux_field) {
$aux_field->setDiff($target);
}
list($changesets, $vs_map, $rendering_references) =
$this->loadChangesetsAndVsMap($diffs, $diff_vs, $target);
@ -336,17 +339,6 @@ class DifferentialRevisionViewController extends DifferentialController {
$handles,
$revision->getCCPHIDs()));
$host = $diff->getSourceMachine();
if ($host) {
$properties['Host'] = phutil_escape_html($host);
}
$path = $diff->getSourcePath();
if ($path) {
$branch = $diff->getBranch() ? ' ('.$diff->getBranch().')' : '';
$properties['Path'] = phutil_escape_html("{$path} {$branch}");
}
$lstar = DifferentialRevisionUpdateHistoryView::renderDiffLintStar($diff);
$lmsg = DifferentialRevisionUpdateHistoryView::getDiffLintMessage($diff);
$ldata = idx($diff_properties, 'arc:lint');
@ -474,18 +466,12 @@ class DifferentialRevisionViewController extends DifferentialController {
$properties['Commits'] = implode('<br />', $links);
}
$properties['Lines'] = number_format($diff->getLineCount());
$arcanist_phid = $diff->getArcanistProjectPHID();
if ($arcanist_phid) {
$properties['Arcanist Project'] = phutil_escape_html(
$handles[$arcanist_phid]->getName());
}
$properties['Apply Patch'] =
'<tt>arc patch D'.$revision->getID().'</tt>';
$properties['Export Patch'] =
'<tt>arc export --revision '.$revision->getID().'</tt>';
return $properties;
}

View file

@ -16,7 +16,8 @@
* limitations under the License.
*/
class DifferentialFieldSpecificationIncompleteException extends Exception {
final class DifferentialFieldSpecificationIncompleteException
extends Exception {
public function __construct(DifferentialFieldSpecification $spec) {
$key = $spec->getStorageKey();

View file

@ -0,0 +1,30 @@
<?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.
*/
final class DifferentialFieldDataNotAvailableException extends Exception {
public function __construct(DifferentialFieldSpecification $spec) {
$key = $spec->getStorageKey();
$class = get_class($spec);
parent::__construct(
"Differential field specification for '{$key}' (of class '{$class}') is ".
"attempting to access data which is not available in this context.");
}
}

View file

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

View file

@ -20,7 +20,13 @@ final class DifferentialDefaultFieldSelector
extends DifferentialFieldSelector {
public function getFieldSpecifications() {
return array();
return array(
new DifferentialHostFieldSpecification(),
new DifferentialPathFieldSpecification(),
new DifferentialLinesFieldSpecification(),
new DifferentialApplyPatchFieldSpecification(),
new DifferentialExportPatchFieldSpecification(),
);
}
}

View file

@ -7,6 +7,11 @@
phutil_require_module('phabricator', 'applications/differential/field/selector/base');
phutil_require_module('phabricator', 'applications/differential/field/specification/applypatch');
phutil_require_module('phabricator', 'applications/differential/field/specification/exportpatch');
phutil_require_module('phabricator', 'applications/differential/field/specification/host');
phutil_require_module('phabricator', 'applications/differential/field/specification/lines');
phutil_require_module('phabricator', 'applications/differential/field/specification/path');
phutil_require_source('DifferentialDefaultFieldSelector.php');

View file

@ -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.
*/
final class DifferentialApplyPatchFieldSpecification
extends DifferentialFieldSpecification {
public function shouldAppearOnRevisionView() {
return true;
}
public function renderLabelForRevisionView() {
return 'Apply Patch:';
}
public function renderValueForRevisionView() {
$revision = $this->getRevision();
return '<tt>arc patch D'.$revision->getID().'</tt>';
}
}

View file

@ -0,0 +1,12 @@
<?php
/**
* This file is automatically generated. Lint this module to rebuild it.
* @generated
*/
phutil_require_module('phabricator', 'applications/differential/field/specification/base');
phutil_require_source('DifferentialApplyPatchFieldSpecification.php');

View file

@ -28,52 +28,79 @@
* @task edit Extending the Revision Edit Interface
* @task view Extending the Revision View Interface
* @task conduit Extending the Conduit View Interface
* @task context Contextual Data
*/
abstract class DifferentialFieldSpecification {
private $revision;
private $diff;
/* -( Storage )------------------------------------------------------------ */
/**
* Return a unique string used to key storage of this field's value, like
* "mycompany.fieldname" or similar.
* "mycompany.fieldname" or similar. You can return null (the default) to
* indicate that this field does not use any storage. This is appropriate for
* display fields, like @{class:DifferentialLinesFieldSpecification}. If you
* implement this, you must also implement @{method:getValueForStorage} and
* @{method:setValueFromStorage}.
*
* @return string Unique key which identifies this field in auxiliary field
* storage. Maximum length is 32.
* @group storage
* @return string|null Unique key which identifies this field in auxiliary
* field storage. Maximum length is 32. Alternatively,
* null (default) to indicate that this field does not
* use auxiliary field storage.
* @task storage
*/
abstract public function getStorageKey();
public function getStorageKey() {
return null;
}
/**
* Return a serialized representation of the field value, appropriate for
* storing in auxiliary field storage.
* storing in auxiliary field storage. You must implement this method if
* you implement @{method:getStorageKey}.
*
* @return string Serialized field value.
* @group storage
* @task storage
*/
abstract public function getValueForStorage();
public function getValueForStorage() {
throw new DifferentialFieldSpecificationIncompleteException($this);
}
/**
* Set the field's value given a serialized storage value. This is called
* when the field is loaded; if no data is available, the value will be
* null.
* null. You must implement this method if you implement
* @{method:getStorageKey}.
*
* @param string|null Serialized field representation (from
* getValueForStorage) or null if no value has ever been
* stored.
* @{method:getValueForStorage}) or null if no value has
* ever been stored.
* @return this
* @group storage
* @task storage
*/
abstract public function setValueFromStorage($value);
public function setValueFromStorage($value) {
throw new DifferentialFieldSpecificationIncompleteException($this);
}
/* -( Extending the Revision Edit Interface )------------------------------ */
/**
* Determine if this field should appear on the "Edit Revision" interface. If
* you return true from this method, you must implement
* @{method:setValueFromRequest}, @{method:renderEditControl} and
* @{method:validateField}.
*
* For a concrete example of a field which implements an edit interface, see
* @{class:DifferentialRevertPlanFieldSpecification}.
*
* @return bool True to indicate that this field implements an edit interface.
* @task edit
*/
public function shouldAppearOnEdit() {
@ -82,6 +109,23 @@ abstract class DifferentialFieldSpecification {
/**
* Set the field's value from an HTTP request. Generally, you should read
* the value of some field name you emitted in @{method:renderEditControl}
* and save it into the object, e.g.:
*
* $this->value = $request->getStr('my-custom-field');
*
* If you have some particularly complicated field, you may need to read
* more data; this is why you have access to the entire request.
*
* You must implement this if you implement @{method:shouldAppearOnEdit}.
*
* You should not perform field validation here; instead, you should implement
* @{method:validateField}.
*
* @param AphrontRequest HTTP request representing a user submitting a form
* with this field in it.
* @return this
* @task edit
*/
public function setValueFromRequest(AphrontRequest $request) {
@ -90,6 +134,21 @@ abstract class DifferentialFieldSpecification {
/**
* Build a renderable object (generally, some @{class:AphrontFormControl})
* which can be appended to a @{class:AphrontFormView} and represents the
* interface the user sees on the "Edit Revision" screen when interacting
* with this field.
*
* For example:
*
* return id(new AphrontFormTextControl())
* ->setLabel('Custom Field')
* ->setName('my-custom-key')
* ->setValue($this->value);
*
* You must implement this if you implement @{method:shouldAppearOnEdit}.
*
* @return AphrontView|string Something renderable.
* @task edit
*/
public function renderEditControl() {
@ -98,10 +157,19 @@ abstract class DifferentialFieldSpecification {
/**
* This method will be called after @{method:setValueFromRequest} but before
* the field is saved. It gives you an opportunity to inspect the field value
* and throw a @{class:DifferentialFieldValidationException} if there is a
* problem with the value the user has provided (for example, the value the
* user entered is not correctly formatted).
*
* By default, fields are not validated.
*
* @return void
* @task edit
*/
public function validateField() {
throw new DifferentialFieldSpecificationIncompleteException($this);
return;
}
@ -149,4 +217,54 @@ abstract class DifferentialFieldSpecification {
throw new DifferentialFieldSpecificationIncompleteException($this);
}
/**
* @task conduit
*/
public function getKeyForConduit() {
$key = $this->getStorageKey();
if ($key === null) {
throw new DifferentialFieldSpecificationIncompleteException($this);
}
return $key;
}
/* -( Contextual Data )---------------------------------------------------- */
/**
* @task context
*/
final public function setRevision(DifferentialRevision $revision) {
$this->revision = $revision;
return $this;
}
/**
* @task context
*/
final public function setDiff(DifferentialDiff $diff) {
$this->diff = $diff;
return $this;
}
/**
* @task context
*/
final protected function getRevision() {
if (empty($this->revision)) {
throw new DifferentialFieldDataNotAvailableException($this);
}
return $this->revision;
}
/**
* @task context
*/
final protected function getDiff() {
if (empty($this->diff)) {
throw new DifferentialFieldDataNotAvailableException($this);
}
return $this->diff;
}
}

View file

@ -7,6 +7,7 @@
phutil_require_module('phabricator', 'applications/differential/field/exception/incomplete');
phutil_require_module('phabricator', 'applications/differential/field/exception/notavailable');
phutil_require_source('DifferentialFieldSpecification.php');

View file

@ -39,7 +39,7 @@ final class DifferentialBlameRevisionFieldSpecification
}
public function setValueFromRequest(AphrontRequest $request) {
$this->value = $request->getStr('aux:phabricator:blame-revision');
$this->value = $request->getStr($this->getStorageKey());
return $this;
}
@ -47,7 +47,7 @@ final class DifferentialBlameRevisionFieldSpecification
return id(new AphrontFormTextControl())
->setLabel('Blame Revision')
->setCaption('Revision which broke the stuff which this change fixes.')
->setName('aux:phabricator:blame-revision')
->setName($this->getStorageKey())
->setValue($this->value);
}

View file

@ -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.
*/
final class DifferentialExportPatchFieldSpecification
extends DifferentialFieldSpecification {
public function shouldAppearOnRevisionView() {
return true;
}
public function renderLabelForRevisionView() {
return 'Export Patch:';
}
public function renderValueForRevisionView() {
$revision = $this->getRevision();
return '<tt>arc export --revision '.$revision->getID().'</tt>';
}
}

View file

@ -0,0 +1,12 @@
<?php
/**
* This file is automatically generated. Lint this module to rebuild it.
* @generated
*/
phutil_require_module('phabricator', 'applications/differential/field/specification/base');
phutil_require_source('DifferentialExportPatchFieldSpecification.php');

View file

@ -0,0 +1,39 @@
<?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.
*/
final class DifferentialHostFieldSpecification
extends DifferentialFieldSpecification {
public function shouldAppearOnRevisionView() {
return true;
}
public function renderLabelForRevisionView() {
return 'Host:';
}
public function renderValueForRevisionView() {
$diff = $this->getDiff();
$host = $diff->getSourceMachine();
if (!$host) {
return null;
}
return phutil_escape_html($host);
}
}

View file

@ -0,0 +1,14 @@
<?php
/**
* This file is automatically generated. Lint this module to rebuild it.
* @generated
*/
phutil_require_module('phabricator', 'applications/differential/field/specification/base');
phutil_require_module('phutil', 'markup');
phutil_require_source('DifferentialHostFieldSpecification.php');

View file

@ -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.
*/
final class DifferentialLinesFieldSpecification
extends DifferentialFieldSpecification {
public function shouldAppearOnRevisionView() {
return true;
}
public function renderLabelForRevisionView() {
return 'Lines:';
}
public function renderValueForRevisionView() {
$diff = $this->getDiff();
return phutil_escape_html(number_format($diff->getLineCount()));
}
}

View file

@ -0,0 +1,14 @@
<?php
/**
* This file is automatically generated. Lint this module to rebuild it.
* @generated
*/
phutil_require_module('phabricator', 'applications/differential/field/specification/base');
phutil_require_module('phutil', 'markup');
phutil_require_source('DifferentialLinesFieldSpecification.php');

View file

@ -0,0 +1,46 @@
<?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.
*/
final class DifferentialPathFieldSpecification
extends DifferentialFieldSpecification {
public function shouldAppearOnRevisionView() {
return true;
}
public function renderLabelForRevisionView() {
return 'Path:';
}
public function renderValueForRevisionView() {
$diff = $this->getDiff();
$path = $diff->getSourcePath();
if (!$path) {
return null;
}
$branch = $diff->getBranch();
if ($branch) {
$branch = ' ('.$branch.')';
}
return phutil_escape_html($path.$branch);
}
}

View file

@ -0,0 +1,14 @@
<?php
/**
* This file is automatically generated. Lint this module to rebuild it.
* @generated
*/
phutil_require_module('phabricator', 'applications/differential/field/specification/base');
phutil_require_module('phutil', 'markup');
phutil_require_source('DifferentialPathFieldSpecification.php');

View file

@ -39,14 +39,14 @@ final class DifferentialRevertPlanFieldSpecification
}
public function setValueFromRequest(AphrontRequest $request) {
$this->value = $request->getStr('aux:phabricator:revert-plan');
$this->value = $request->getStr($this->getStorageKey());
return $this;
}
public function renderEditControl() {
return id(new AphrontFormTextAreaControl())
->setLabel('Revert Plan')
->setName('aux:phabricator:revert-plan')
->setName($this->getStorageKey())
->setCaption('Special steps required to safely revert this change.')
->setValue($this->value);
}

View file

@ -47,6 +47,7 @@ final class DifferentialAuxiliaryField extends DifferentialDAO {
}
foreach ($aux_fields as $aux_field) {
$aux_field->setRevision($revision);
$key = $aux_field->getStorageKey();
if ($key) {
$aux_field->setValueFromStorage(idx($field_data, $key));