From 9e4c16c4c3ac3155665b2dbbc81f593576d61d57 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 16 Dec 2016 09:23:07 -0800 Subject: [PATCH] Remove Differential "Title" custom field Summary: Ref T11114. Obsoleted by Modular Transactions + EditEngine + CommitMessageField + we just "hard code" the title of revisions into the page because we're craaazy. Test Plan: - Made an edit on `stable`. - Viewed the edit on this change, it still had the proper UI strings. - Edited/created/updated revisions. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11114 Differential Revision: https://secure.phabricator.com/D17083 --- src/__phutil_library_map__.php | 2 - .../PhabricatorDifferentialConfigOptions.php | 1 - .../customfield/DifferentialTitleField.php | 125 ------------------ .../storage/DifferentialTransaction.php | 17 +++ .../storage/PhabricatorModularTransaction.php | 6 +- 5 files changed, 22 insertions(+), 129 deletions(-) delete mode 100644 src/applications/differential/customfield/DifferentialTitleField.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 8abf50a49f..31fe6f0817 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -572,7 +572,6 @@ phutil_register_library_map(array( 'DifferentialTestPlanCommitMessageField' => 'applications/differential/field/DifferentialTestPlanCommitMessageField.php', 'DifferentialTestPlanField' => 'applications/differential/customfield/DifferentialTestPlanField.php', 'DifferentialTitleCommitMessageField' => 'applications/differential/field/DifferentialTitleCommitMessageField.php', - 'DifferentialTitleField' => 'applications/differential/customfield/DifferentialTitleField.php', 'DifferentialTransaction' => 'applications/differential/storage/DifferentialTransaction.php', 'DifferentialTransactionComment' => 'applications/differential/storage/DifferentialTransactionComment.php', 'DifferentialTransactionEditor' => 'applications/differential/editor/DifferentialTransactionEditor.php', @@ -5236,7 +5235,6 @@ phutil_register_library_map(array( 'DifferentialTestPlanCommitMessageField' => 'DifferentialCommitMessageField', 'DifferentialTestPlanField' => 'DifferentialCoreCustomField', 'DifferentialTitleCommitMessageField' => 'DifferentialCommitMessageField', - 'DifferentialTitleField' => 'DifferentialCoreCustomField', 'DifferentialTransaction' => 'PhabricatorModularTransaction', 'DifferentialTransactionComment' => 'PhabricatorApplicationTransactionComment', 'DifferentialTransactionEditor' => 'PhabricatorApplicationTransactionEditor', diff --git a/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php b/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php index 8f61f235b0..a3db26bef6 100644 --- a/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php +++ b/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php @@ -25,7 +25,6 @@ final class PhabricatorDifferentialConfigOptions $custom_field_type = 'custom:PhabricatorCustomFieldConfigOptionType'; $fields = array( - new DifferentialTitleField(), new DifferentialSummaryField(), new DifferentialTestPlanField(), new DifferentialReviewersField(), diff --git a/src/applications/differential/customfield/DifferentialTitleField.php b/src/applications/differential/customfield/DifferentialTitleField.php deleted file mode 100644 index 82d6ddc1f3..0000000000 --- a/src/applications/differential/customfield/DifferentialTitleField.php +++ /dev/null @@ -1,125 +0,0 @@ ->'); - } - - protected function readValueFromRevision( - DifferentialRevision $revision) { - return $revision->getTitle(); - } - - protected function writeValueToRevision( - DifferentialRevision $revision, - $value) { - $revision->setTitle($value); - } - - protected function getCoreFieldRequiredErrorString() { - return pht('You must choose a title for this revision.'); - } - - public function readValueFromRequest(AphrontRequest $request) { - $this->setValue($request->getStr($this->getFieldKey())); - } - - protected function isCoreFieldRequired() { - return true; - } - - public function renderEditControl(array $handles) { - return id(new AphrontFormTextAreaControl()) - ->setHeight(AphrontFormTextAreaControl::HEIGHT_VERY_SHORT) - ->setName($this->getFieldKey()) - ->setValue($this->getValue()) - ->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) { - - $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)); - } - } - - public function shouldAppearInCommitMessage() { - return true; - } - - public function shouldOverwriteWhenCommitMessageIsEdited() { - return true; - } - - public function validateCommitMessageValue($value) { - if (!strlen($value)) { - throw new DifferentialFieldValidationException( - pht( - 'You must provide a revision title in the first line '. - 'of your commit message.')); - } - - if (preg_match('/^<<.*>>$/', $value)) { - throw new DifferentialFieldValidationException( - pht( - 'Replace the line "%s" with a human-readable revision title which '. - 'describes the changes you are making.', - self::getDefaultTitle())); - } - } - -} diff --git a/src/applications/differential/storage/DifferentialTransaction.php b/src/applications/differential/storage/DifferentialTransaction.php index 0c9e0589eb..0ea2731354 100644 --- a/src/applications/differential/storage/DifferentialTransaction.php +++ b/src/applications/differential/storage/DifferentialTransaction.php @@ -22,6 +22,23 @@ final class DifferentialTransaction return 'DifferentialRevisionTransactionType'; } + protected function newFallbackModularTransactionType() { + // TODO: This allows us to render modern strings for older transactions + // without doing a migration. At some point, we should do a migration and + // throw this away. + + $xaction_type = $this->getTransactionType(); + if ($xaction_type == PhabricatorTransactions::TYPE_CUSTOMFIELD) { + switch ($this->getMetadataValue('customfield:key')) { + case 'differential:title': + return new DifferentialRevisionTitleTransaction(); + } + } + + return parent::newFallbackModularTransactionType(); + } + + public function setIsCommandeerSideEffect($is_side_effect) { $this->isCommandeerSideEffect = $is_side_effect; return $this; diff --git a/src/applications/transactions/storage/PhabricatorModularTransaction.php b/src/applications/transactions/storage/PhabricatorModularTransaction.php index a1da48df33..035a2585fd 100644 --- a/src/applications/transactions/storage/PhabricatorModularTransaction.php +++ b/src/applications/transactions/storage/PhabricatorModularTransaction.php @@ -46,7 +46,7 @@ abstract class PhabricatorModularTransaction $key = $this->getTransactionType(); if (empty($types[$key])) { - $type = new PhabricatorCoreVoidTransaction(); + $type = $this->newFallbackModularTransactionType(); } else { $type = clone $types[$key]; } @@ -56,6 +56,10 @@ abstract class PhabricatorModularTransaction return $type; } + protected function newFallbackModularTransactionType() { + return new PhabricatorCoreVoidTransaction(); + } + final public function generateOldValue($object) { return $this->getTransactionImplementation()->generateOldValue($object); }