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

Implement "Pro" version of revision editor, with one field

Summary:
Ref T3886. I spent a few hours trying to make `DifferentialFieldSpecification` extend `PhabricatorCustomField` so I could be more blunt in my approach here and just swap the whole thing over in one go (more or less like I did with Maniphest) but we have a ton of custom fields and things felt really shaky and the change was enormous and hard to keep track of.

Instead, I'm going to do this more gradually and go field-by-field. This implements a CustomField version of the "Title" field.

(There are no links to this in the UI.)

Test Plan:
{F115353}

{F115354}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3886

Differential Revision: https://secure.phabricator.com/D8276
This commit is contained in:
epriestley 2014-02-18 16:32:55 -08:00
parent 79fa0b0397
commit 65bc2b1ac5
7 changed files with 446 additions and 1 deletions

View file

@ -355,6 +355,7 @@ phutil_register_library_map(array(
'DifferentialCommitsFieldSpecification' => 'applications/differential/field/specification/DifferentialCommitsFieldSpecification.php', 'DifferentialCommitsFieldSpecification' => 'applications/differential/field/specification/DifferentialCommitsFieldSpecification.php',
'DifferentialConflictsFieldSpecification' => 'applications/differential/field/specification/DifferentialConflictsFieldSpecification.php', 'DifferentialConflictsFieldSpecification' => 'applications/differential/field/specification/DifferentialConflictsFieldSpecification.php',
'DifferentialController' => 'applications/differential/controller/DifferentialController.php', 'DifferentialController' => 'applications/differential/controller/DifferentialController.php',
'DifferentialCustomField' => 'applications/differential/customfield/DifferentialCustomField.php',
'DifferentialCustomFieldDependsOnParser' => 'applications/differential/field/parser/DifferentialCustomFieldDependsOnParser.php', 'DifferentialCustomFieldDependsOnParser' => 'applications/differential/field/parser/DifferentialCustomFieldDependsOnParser.php',
'DifferentialCustomFieldDependsOnParserTestCase' => 'applications/differential/field/parser/__tests__/DifferentialCustomFieldDependsOnParserTestCase.php', 'DifferentialCustomFieldDependsOnParserTestCase' => 'applications/differential/field/parser/__tests__/DifferentialCustomFieldDependsOnParserTestCase.php',
'DifferentialCustomFieldNumericIndex' => 'applications/differential/storage/DifferentialCustomFieldNumericIndex.php', 'DifferentialCustomFieldNumericIndex' => 'applications/differential/storage/DifferentialCustomFieldNumericIndex.php',
@ -444,6 +445,7 @@ phutil_register_library_map(array(
'DifferentialRevisionDetailRenderer' => 'applications/differential/controller/DifferentialRevisionDetailRenderer.php', 'DifferentialRevisionDetailRenderer' => 'applications/differential/controller/DifferentialRevisionDetailRenderer.php',
'DifferentialRevisionDetailView' => 'applications/differential/view/DifferentialRevisionDetailView.php', 'DifferentialRevisionDetailView' => 'applications/differential/view/DifferentialRevisionDetailView.php',
'DifferentialRevisionEditController' => 'applications/differential/controller/DifferentialRevisionEditController.php', 'DifferentialRevisionEditController' => 'applications/differential/controller/DifferentialRevisionEditController.php',
'DifferentialRevisionEditControllerPro' => 'applications/differential/controller/DifferentialRevisionEditControllerPro.php',
'DifferentialRevisionEditor' => 'applications/differential/editor/DifferentialRevisionEditor.php', 'DifferentialRevisionEditor' => 'applications/differential/editor/DifferentialRevisionEditor.php',
'DifferentialRevisionIDFieldParserTestCase' => 'applications/differential/field/specification/__tests__/DifferentialRevisionIDFieldParserTestCase.php', 'DifferentialRevisionIDFieldParserTestCase' => 'applications/differential/field/specification/__tests__/DifferentialRevisionIDFieldParserTestCase.php',
'DifferentialRevisionIDFieldSpecification' => 'applications/differential/field/specification/DifferentialRevisionIDFieldSpecification.php', 'DifferentialRevisionIDFieldSpecification' => 'applications/differential/field/specification/DifferentialRevisionIDFieldSpecification.php',
@ -462,9 +464,11 @@ phutil_register_library_map(array(
'DifferentialSummaryFieldSpecification' => 'applications/differential/field/specification/DifferentialSummaryFieldSpecification.php', 'DifferentialSummaryFieldSpecification' => 'applications/differential/field/specification/DifferentialSummaryFieldSpecification.php',
'DifferentialTasksAttacher' => 'applications/differential/DifferentialTasksAttacher.php', 'DifferentialTasksAttacher' => 'applications/differential/DifferentialTasksAttacher.php',
'DifferentialTestPlanFieldSpecification' => 'applications/differential/field/specification/DifferentialTestPlanFieldSpecification.php', 'DifferentialTestPlanFieldSpecification' => 'applications/differential/field/specification/DifferentialTestPlanFieldSpecification.php',
'DifferentialTitleField' => 'applications/differential/customfield/DifferentialTitleField.php',
'DifferentialTitleFieldSpecification' => 'applications/differential/field/specification/DifferentialTitleFieldSpecification.php', 'DifferentialTitleFieldSpecification' => 'applications/differential/field/specification/DifferentialTitleFieldSpecification.php',
'DifferentialTransaction' => 'applications/differential/storage/DifferentialTransaction.php', 'DifferentialTransaction' => 'applications/differential/storage/DifferentialTransaction.php',
'DifferentialTransactionComment' => 'applications/differential/storage/DifferentialTransactionComment.php', 'DifferentialTransactionComment' => 'applications/differential/storage/DifferentialTransactionComment.php',
'DifferentialTransactionEditor' => 'applications/differential/editor/DifferentialTransactionEditor.php',
'DifferentialTransactionQuery' => 'applications/differential/query/DifferentialTransactionQuery.php', 'DifferentialTransactionQuery' => 'applications/differential/query/DifferentialTransactionQuery.php',
'DifferentialTransactionView' => 'applications/differential/view/DifferentialTransactionView.php', 'DifferentialTransactionView' => 'applications/differential/view/DifferentialTransactionView.php',
'DifferentialUnitFieldSpecification' => 'applications/differential/field/specification/DifferentialUnitFieldSpecification.php', 'DifferentialUnitFieldSpecification' => 'applications/differential/field/specification/DifferentialUnitFieldSpecification.php',
@ -2887,6 +2891,7 @@ phutil_register_library_map(array(
'DifferentialCommitsFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialCommitsFieldSpecification' => 'DifferentialFieldSpecification',
'DifferentialConflictsFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialConflictsFieldSpecification' => 'DifferentialFieldSpecification',
'DifferentialController' => 'PhabricatorController', 'DifferentialController' => 'PhabricatorController',
'DifferentialCustomField' => 'PhabricatorCustomField',
'DifferentialCustomFieldDependsOnParser' => 'PhabricatorCustomFieldMonogramParser', 'DifferentialCustomFieldDependsOnParser' => 'PhabricatorCustomFieldMonogramParser',
'DifferentialCustomFieldDependsOnParserTestCase' => 'PhabricatorTestCase', 'DifferentialCustomFieldDependsOnParserTestCase' => 'PhabricatorTestCase',
'DifferentialCustomFieldNumericIndex' => 'PhabricatorCustomFieldNumericIndexStorage', 'DifferentialCustomFieldNumericIndex' => 'PhabricatorCustomFieldNumericIndexStorage',
@ -2975,9 +2980,11 @@ phutil_register_library_map(array(
4 => 'PhrequentTrackableInterface', 4 => 'PhrequentTrackableInterface',
5 => 'HarbormasterBuildableInterface', 5 => 'HarbormasterBuildableInterface',
6 => 'PhabricatorSubscribableInterface', 6 => 'PhabricatorSubscribableInterface',
7 => 'PhabricatorCustomFieldInterface',
), ),
'DifferentialRevisionDetailView' => 'AphrontView', 'DifferentialRevisionDetailView' => 'AphrontView',
'DifferentialRevisionEditController' => 'DifferentialController', 'DifferentialRevisionEditController' => 'DifferentialController',
'DifferentialRevisionEditControllerPro' => 'DifferentialController',
'DifferentialRevisionEditor' => 'PhabricatorEditor', 'DifferentialRevisionEditor' => 'PhabricatorEditor',
'DifferentialRevisionIDFieldParserTestCase' => 'PhabricatorTestCase', 'DifferentialRevisionIDFieldParserTestCase' => 'PhabricatorTestCase',
'DifferentialRevisionIDFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialRevisionIDFieldSpecification' => 'DifferentialFieldSpecification',
@ -2998,9 +3005,11 @@ phutil_register_library_map(array(
'DifferentialSubscribeController' => 'DifferentialController', 'DifferentialSubscribeController' => 'DifferentialController',
'DifferentialSummaryFieldSpecification' => 'DifferentialFreeformFieldSpecification', 'DifferentialSummaryFieldSpecification' => 'DifferentialFreeformFieldSpecification',
'DifferentialTestPlanFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialTestPlanFieldSpecification' => 'DifferentialFieldSpecification',
'DifferentialTitleField' => 'DifferentialCustomField',
'DifferentialTitleFieldSpecification' => 'DifferentialFreeformFieldSpecification', 'DifferentialTitleFieldSpecification' => 'DifferentialFreeformFieldSpecification',
'DifferentialTransaction' => 'PhabricatorApplicationTransaction', 'DifferentialTransaction' => 'PhabricatorApplicationTransaction',
'DifferentialTransactionComment' => 'PhabricatorApplicationTransactionComment', 'DifferentialTransactionComment' => 'PhabricatorApplicationTransactionComment',
'DifferentialTransactionEditor' => 'PhabricatorApplicationTransactionEditor',
'DifferentialTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'DifferentialTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
'DifferentialTransactionView' => 'PhabricatorApplicationTransactionView', 'DifferentialTransactionView' => 'PhabricatorApplicationTransactionView',
'DifferentialUnitFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialUnitFieldSpecification' => 'DifferentialFieldSpecification',

View file

@ -49,6 +49,8 @@ final class PhabricatorApplicationDifferential extends PhabricatorApplication {
'changeset/' => 'DifferentialChangesetViewController', 'changeset/' => 'DifferentialChangesetViewController',
'revision/edit/(?:(?P<id>[1-9]\d*)/)?' 'revision/edit/(?:(?P<id>[1-9]\d*)/)?'
=> 'DifferentialRevisionEditController', => 'DifferentialRevisionEditController',
'revision/editpro/(?:(?P<id>[1-9]\d*)/)?'
=> 'DifferentialRevisionEditControllerPro',
'revision/land/(?:(?P<id>[1-9]\d*))/(?P<strategy>[^/]+)/' 'revision/land/(?:(?P<id>[1-9]\d*))/(?P<strategy>[^/]+)/'
=> 'DifferentialRevisionLandController', => 'DifferentialRevisionLandController',
'comment/' => array( 'comment/' => array(

View file

@ -0,0 +1,159 @@
<?php
final class DifferentialRevisionEditControllerPro
extends DifferentialController {
private $id;
public function willProcessRequest(array $data) {
$this->id = idx($data, 'id');
}
public function processRequest() {
$request = $this->getRequest();
$viewer = $request->getUser();
if (!$this->id) {
$this->id = $request->getInt('revisionID');
}
if ($this->id) {
$revision = id(new DifferentialRevisionQuery())
->setViewer($viewer)
->withIDs(array($this->id))
->needRelationships(true)
->needReviewerStatus(true)
->requireCapabilities(
array(
PhabricatorPolicyCapability::CAN_VIEW,
PhabricatorPolicyCapability::CAN_EDIT,
))
->executeOne();
if (!$revision) {
return new Aphront404Response();
}
} else {
$revision = DifferentialRevision::initializeNewRevision($viewer);
}
$diff_id = $request->getInt('diffID');
if ($diff_id) {
$diff = id(new DifferentialDiffQuery())
->setViewer($viewer)
->withIDs(array($diff_id))
->executeOne();
if (!$diff) {
return new Aphront404Response();
}
if ($diff->getRevisionID()) {
// TODO: Redirect?
throw new Exception("This diff is already attached to a revision!");
}
} else {
$diff = null;
}
$field_list = PhabricatorCustomField::getObjectFields(
$revision,
PhabricatorCustomField::ROLE_EDIT);
$field_list
->setViewer($viewer)
->readFieldsFromStorage($revision);
$validation_exception = null;
if ($request->isFormPost() && !$request->getStr('viaDiffView')) {
$xactions = $field_list->buildFieldTransactionsFromRequest(
new DifferentialTransaction(),
$request);
$editor = id(new DifferentialTransactionEditor())
->setActor($viewer)
->setContentSourceFromRequest($request)
->setContinueOnNoEffect(true);
try {
$editor->applyTransactions($revision, $xactions);
$revision_uri = '/D'.$revision->getID();
return id(new AphrontRedirectResponse())->setURI($revision_uri);
} catch (PhabricatorApplicationTransactionValidationException $ex) {
$validation_exception = $ex;
}
}
$form = new AphrontFormView();
$form->setUser($request->getUser());
if ($diff) {
$form->addHiddenInput('diffID', $diff->getID());
}
if ($revision->getID()) {
$form->setAction(
'/differential/revision/editpro/'.$revision->getID().'/');
} else {
$form->setAction('/differential/revision/editpro/');
}
if ($diff && $revision->getID()) {
$form
->appendChild(
id(new AphrontFormTextAreaControl())
->setLabel(pht('Comments'))
->setName('comments')
->setCaption(pht("Explain what's new in this diff."))
->setValue($request->getStr('comments')))
->appendChild(
id(new AphrontFormSubmitControl())
->setValue(pht('Save')))
->appendChild(
id(new AphrontFormDividerControl()));
}
$field_list->appendFieldsToForm($form);
$submit = id(new AphrontFormSubmitControl())
->setValue('Save');
if ($diff) {
$submit->addCancelButton('/differential/diff/'.$diff->getID().'/');
} else {
$submit->addCancelButton('/D'.$revision->getID());
}
$form->appendChild($submit);
$crumbs = $this->buildApplicationCrumbs();
if ($revision->getID()) {
if ($diff) {
$title = pht('Update Differential Revision');
$crumbs->addTextCrumb(
'D'.$revision->getID(),
'/differential/diff/'.$diff->getID().'/');
} else {
$title = pht('Edit Differential Revision');
$crumbs->addTextCrumb(
'D'.$revision->getID(),
'/D'.$revision->getID());
}
} else {
$title = pht('Create New Differential Revision');
}
$form_box = id(new PHUIObjectBoxView())
->setHeaderText($title)
->setValidationException($validation_exception)
->setForm($form);
$crumbs->addTextCrumb($title);
return $this->buildApplicationPage(
array(
$crumbs,
$form_box,
),
array(
'title' => $title,
'device' => true,
));
}
}

View file

@ -0,0 +1,6 @@
<?php
abstract class DifferentialCustomField
extends PhabricatorCustomField {
}

View file

@ -0,0 +1,142 @@
<?php
final class DifferentialTitleField
extends DifferentialCustomField {
private $value;
private $fieldError = true;
public function setFieldError($field_error) {
$this->fieldError = $field_error;
return $this;
}
public function getFieldError() {
return $this->fieldError;
}
public function getFieldKey() {
return 'differential:title';
}
public function getFieldName() {
return pht('Title');
}
public function getFieldDescription() {
return pht('Stores the revision title.');
}
public function canDisableField() {
return false;
}
public function shouldAppearInApplicationTransactions() {
return true;
}
public function shouldAppearInEditView() {
return true;
}
protected function didSetObject(PhabricatorCustomFieldInterface $object) {
$this->value = $object->getTitle();
}
public function getOldValueForApplicationTransactions() {
return $this->getObject()->getTitle();
}
public function getNewValueForApplicationTransactions() {
return $this->value;
}
public function validateApplicationTransactions(
PhabricatorApplicationTransactionEditor $editor,
$type,
array $xactions) {
$errors = parent::validateApplicationTransactions(
$editor,
$type,
$xactions);
$transaction = null;
foreach ($xactions as $xaction) {
$value = $xaction->getNewValue();
if (!strlen($value)) {
$error = new PhabricatorApplicationTransactionValidationError(
$type,
pht('Required'),
pht('You must choose a title for this revision.'),
$xaction);
$error->setIsMissingFieldError(true);
$errors[] = $error;
$this->setFieldError(pht('Required'));
}
}
}
public function applyApplicationTransactionInternalEffects(
PhabricatorApplicationTransaction $xaction) {
$this->getObject()->setTitle($xaction->getNewValue());
}
public function readValueFromRequest(AphrontRequest $request) {
$this->value = $request->getStr($this->getFieldKey());
}
public function renderEditControl() {
return id(new AphrontFormTextAreaControl())
->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_SHORT)
->setName($this->getFieldKey())
->setValue($this->value)
->setError($this->getFieldError())
->setLabel($this->getFieldName());
}
public function getApplicationTransactionTitle(
PhabricatorApplicationTransaction $xaction) {
$author_phid = $xaction->getAuthorPHID();
$old = $xaction->getOldValue();
$new = $xaction->getNewValue();
if (strlen($old)) {
return pht(
'%s retitled this revision from "%s" to "%s".',
$xaction->renderHandleLink($author_phid),
$old,
$new);
} else {
return pht(
'%s created this revision.',
$xaction->renderHandleLink($author_phid));
}
}
public function getApplicationTransactionTitleForFeed(
PhabricatorApplicationTransaction $xaction,
PhabricatorFeedStory $story) {
$object_phid = $xaction->getObjectPHID();
$author_phid = $xaction->getAuthorPHID();
$old = $xaction->getOldValue();
$new = $xaction->getNewValue();
if (strlen($old)) {
return pht(
'%s retitled %s, from "%s" to "%s".',
$xaction->renderHandleLink($author_phid),
$xaction->renderHandleLink($object_phid),
$old,
$new);
} else {
return pht(
'%s created %s.',
$xaction->renderHandleLink($author_phid),
$xaction->renderHandleLink($object_phid));
}
}
}

View file

@ -0,0 +1,100 @@
<?php
final class DifferentialTransactionEditor
extends PhabricatorApplicationTransactionEditor {
public function getTransactionTypes() {
$types = parent::getTransactionTypes();
/*
$types[] = PhabricatorTransactions::TYPE_EDGE;
$types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
$types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
$types[] = DifferentialTransaction::TYPE_INLINE;
$types[] = DifferentialTransaction::TYPE_UPDATE;
$types[] = DifferentialTransaction::TYPE_ACTION;
*/
return $types;
}
protected function getCustomTransactionOldValue(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
}
return parent::getCustomTransactionOldValue($object, $xaction);
}
protected function getCustomTransactionNewValue(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
}
return parent::getCustomTransactionNewValue($object, $xaction);
}
protected function applyCustomInternalTransaction(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
}
return parent::applyCustomInternalTransaction($object, $xaction);
}
protected function applyCustomExternalTransaction(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
}
return parent::applyCustomExternalTransaction($object, $xaction);
}
protected function validateTransaction(
PhabricatorLiskDAO $object,
$type,
array $xactions) {
$errors = parent::validateTransaction($object, $type, $xactions);
switch ($type) {
}
return $errors;
}
protected function requireCapabilities(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
}
return parent::requireCapabilities($object, $xaction);
}
protected function supportsSearch() {
return true;
}
protected function extractFilePHIDsFromCustomTransaction(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
switch ($xaction->getTransactionType()) {
}
return parent::extractFilePHIDsFromCustomTransaction($object, $xaction);
}
}

View file

@ -7,7 +7,8 @@ final class DifferentialRevision extends DifferentialDAO
PhabricatorFlaggableInterface, PhabricatorFlaggableInterface,
PhrequentTrackableInterface, PhrequentTrackableInterface,
HarbormasterBuildableInterface, HarbormasterBuildableInterface,
PhabricatorSubscribableInterface { PhabricatorSubscribableInterface,
PhabricatorCustomFieldInterface {
protected $title = ''; protected $title = '';
protected $originalTitle; protected $originalTitle;
@ -39,6 +40,7 @@ final class DifferentialRevision extends DifferentialDAO
private $repository = self::ATTACHABLE; private $repository = self::ATTACHABLE;
private $reviewerStatus = self::ATTACHABLE; private $reviewerStatus = self::ATTACHABLE;
private $customFields = self::ATTACHABLE;
const TABLE_COMMIT = 'differential_commit'; const TABLE_COMMIT = 'differential_commit';
@ -436,4 +438,29 @@ final class DifferentialRevision extends DifferentialDAO
return false; return false;
} }
/* -( PhabricatorCustomFieldInterface )------------------------------------ */
public function getCustomFieldSpecificationForRole($role) {
return array_fill_keys(
array(
),
array('disabled' => false));
}
public function getCustomFieldBaseClass() {
return 'DifferentialCustomField';
}
public function getCustomFields() {
return $this->assertAttached($this->customFields);
}
public function attachCustomFields(PhabricatorCustomFieldAttachment $fields) {
$this->customFields = $fields;
return $this;
}
} }