Remove some remnants of the old ways commit mesage fields worked from Differential
Summary: Ref T11114. Ref T12085. I missed a few pieces of cleanup when moving all this stuff over. In particular, load all fields which use Custom Field storage before doing commit-message-related stuff, instead of just the ones that claim they appear on commit messages. Test Plan: Edited revisions and made API calls without apparent issues. See followup on T12085, shortly. Reviewers: chad Reviewed By: chad Maniphest Tasks: T12085, T11114 Differential Revision: https://secure.phabricator.com/D17207
This commit is contained in:
		@@ -142,9 +142,11 @@ final class DifferentialGetCommitMessageConduitAPIMethod
 | 
			
		||||
  private function loadCustomFieldStorage(
 | 
			
		||||
    PhabricatorUser $viewer,
 | 
			
		||||
    DifferentialRevision $revision) {
 | 
			
		||||
 | 
			
		||||
    $custom_field_list = PhabricatorCustomField::getObjectFields(
 | 
			
		||||
      $revision,
 | 
			
		||||
      DifferentialCustomField::ROLE_COMMITMESSAGE);
 | 
			
		||||
      PhabricatorCustomField::ROLE_STORAGE);
 | 
			
		||||
 | 
			
		||||
    $custom_field_list
 | 
			
		||||
      ->setViewer($viewer)
 | 
			
		||||
      ->readFieldsFromStorage($revision);
 | 
			
		||||
 
 | 
			
		||||
@@ -36,10 +36,6 @@ final class DifferentialAuditorsField
 | 
			
		||||
    return true;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function shouldAppearInCommitMessage() {
 | 
			
		||||
    return true;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function shouldAppearInConduitTransactions() {
 | 
			
		||||
    return true;
 | 
			
		||||
  }
 | 
			
		||||
 
 | 
			
		||||
@@ -91,10 +91,6 @@ final class DifferentialBlameRevisionField
 | 
			
		||||
      $xaction->renderHandleLink($object_phid));
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function shouldAppearInCommitMessage() {
 | 
			
		||||
    return true;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function shouldAppearInConduitDictionary() {
 | 
			
		||||
    return true;
 | 
			
		||||
  }
 | 
			
		||||
 
 | 
			
		||||
@@ -156,15 +156,6 @@ abstract class DifferentialCoreCustomField
 | 
			
		||||
    return $this->value;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function readValueFromCommitMessage($value) {
 | 
			
		||||
    $this->setValue($value);
 | 
			
		||||
    return $this;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function renderCommitMessageValue(array $handles) {
 | 
			
		||||
    return $this->getValue();
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function getConduitDictionaryValue() {
 | 
			
		||||
    return $this->getValue();
 | 
			
		||||
  }
 | 
			
		||||
 
 | 
			
		||||
@@ -7,9 +7,6 @@
 | 
			
		||||
abstract class DifferentialCustomField
 | 
			
		||||
  extends PhabricatorCustomField {
 | 
			
		||||
 | 
			
		||||
  const ROLE_COMMITMESSAGE      = 'differential:commitmessage';
 | 
			
		||||
  const ROLE_COMMITMESSAGEEDIT  = 'differential:commitmessageedit';
 | 
			
		||||
 | 
			
		||||
  /**
 | 
			
		||||
   * TODO: It would be nice to remove this, but a lot of different code is
 | 
			
		||||
   * bound together by it. Until everything is modernized, retaining the old
 | 
			
		||||
@@ -25,18 +22,6 @@ abstract class DifferentialCustomField
 | 
			
		||||
    return $this->getFieldKeyForConduit();
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function shouldEnableForRole($role) {
 | 
			
		||||
    switch ($role) {
 | 
			
		||||
      case self::ROLE_COMMITMESSAGE:
 | 
			
		||||
        return $this->shouldAppearInCommitMessage();
 | 
			
		||||
      case self::ROLE_COMMITMESSAGEEDIT:
 | 
			
		||||
        return $this->shouldAppearInCommitMessage() &&
 | 
			
		||||
               $this->shouldAllowEditInCommitMessage();
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    return parent::shouldEnableForRole($role);
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  protected function parseObjectList(
 | 
			
		||||
    $value,
 | 
			
		||||
    array $types,
 | 
			
		||||
@@ -97,39 +82,6 @@ abstract class DifferentialCustomField
 | 
			
		||||
/* -(  Integration with Commit Messages  )----------------------------------- */
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
  /**
 | 
			
		||||
   * @task commitmessage
 | 
			
		||||
   */
 | 
			
		||||
  public function shouldAppearInCommitMessage() {
 | 
			
		||||
    if ($this->getProxy()) {
 | 
			
		||||
      return $this->getProxy()->shouldAppearInCommitMessage();
 | 
			
		||||
    }
 | 
			
		||||
    return false;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
  /**
 | 
			
		||||
   * @task commitmessage
 | 
			
		||||
   */
 | 
			
		||||
  public function shouldAppearInCommitMessageTemplate() {
 | 
			
		||||
    if ($this->getProxy()) {
 | 
			
		||||
      return $this->getProxy()->shouldAppearInCommitMessageTemplate();
 | 
			
		||||
    }
 | 
			
		||||
    return false;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
  /**
 | 
			
		||||
   * @task commitmessage
 | 
			
		||||
   */
 | 
			
		||||
  public function shouldAllowEditInCommitMessage() {
 | 
			
		||||
    if ($this->getProxy()) {
 | 
			
		||||
      return $this->getProxy()->shouldAllowEditInCommitMessage();
 | 
			
		||||
    }
 | 
			
		||||
    return true;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
  /**
 | 
			
		||||
   * @task commitmessage
 | 
			
		||||
   */
 | 
			
		||||
@@ -141,95 +93,6 @@ abstract class DifferentialCustomField
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
  /**
 | 
			
		||||
   * @task commitmessage
 | 
			
		||||
   */
 | 
			
		||||
  public function getCommitMessageLabels() {
 | 
			
		||||
    if ($this->getProxy()) {
 | 
			
		||||
      return $this->getProxy()->getCommitMessageLabels();
 | 
			
		||||
    }
 | 
			
		||||
    return array($this->renderCommitMessageLabel());
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
  /**
 | 
			
		||||
   * @task commitmessage
 | 
			
		||||
   */
 | 
			
		||||
  public function parseValueFromCommitMessage($value) {
 | 
			
		||||
    if ($this->getProxy()) {
 | 
			
		||||
      return $this->getProxy()->parseValueFromCommitMessage($value);
 | 
			
		||||
    }
 | 
			
		||||
    return $value;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
  /**
 | 
			
		||||
   * @task commitmessage
 | 
			
		||||
   */
 | 
			
		||||
  public function readValueFromCommitMessage($value) {
 | 
			
		||||
    if ($this->getProxy()) {
 | 
			
		||||
      $this->getProxy()->readValueFromCommitMessage($value);
 | 
			
		||||
      return $this;
 | 
			
		||||
    }
 | 
			
		||||
    return $this;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
  /**
 | 
			
		||||
   * @task commitmessage
 | 
			
		||||
   */
 | 
			
		||||
  public function shouldOverwriteWhenCommitMessageIsEdited() {
 | 
			
		||||
    if ($this->getProxy()) {
 | 
			
		||||
      return $this->getProxy()->shouldOverwriteWhenCommitMessageIsEdited();
 | 
			
		||||
    }
 | 
			
		||||
    return false;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
  /**
 | 
			
		||||
   * @task commitmessage
 | 
			
		||||
   */
 | 
			
		||||
  public function getRequiredHandlePHIDsForCommitMessage() {
 | 
			
		||||
    if ($this->getProxy()) {
 | 
			
		||||
      return $this->getProxy()->getRequiredHandlePHIDsForCommitMessage();
 | 
			
		||||
    }
 | 
			
		||||
    return array();
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
  /**
 | 
			
		||||
   * @task commitmessage
 | 
			
		||||
   */
 | 
			
		||||
  public function renderCommitMessageLabel() {
 | 
			
		||||
    if ($this->getProxy()) {
 | 
			
		||||
      return $this->getProxy()->renderCommitMessageLabel();
 | 
			
		||||
    }
 | 
			
		||||
    return $this->getFieldName();
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
  /**
 | 
			
		||||
   * @task commitmessage
 | 
			
		||||
   */
 | 
			
		||||
  public function renderCommitMessageValue(array $handles) {
 | 
			
		||||
    if ($this->getProxy()) {
 | 
			
		||||
      return $this->getProxy()->renderCommitMessageValue($handles);
 | 
			
		||||
    }
 | 
			
		||||
    throw new PhabricatorCustomFieldImplementationIncompleteException($this);
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
  /**
 | 
			
		||||
   * @task commitmessage
 | 
			
		||||
   */
 | 
			
		||||
  public function validateCommitMessageValue($value) {
 | 
			
		||||
    if ($this->getProxy()) {
 | 
			
		||||
      return $this->getProxy()->validateCommitMessageValue($value);
 | 
			
		||||
    }
 | 
			
		||||
    return;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
/* -(  Integration with Diff Properties  )----------------------------------- */
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
@@ -270,39 +270,6 @@ final class DifferentialJIRAIssuesField
 | 
			
		||||
    $editor->save();
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function shouldAppearInCommitMessage() {
 | 
			
		||||
    return true;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function shouldAppearInCommitMessageTemplate() {
 | 
			
		||||
    return true;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function getCommitMessageLabels() {
 | 
			
		||||
    return array(
 | 
			
		||||
      'JIRA',
 | 
			
		||||
      'JIRA Issues',
 | 
			
		||||
      'JIRA Issue',
 | 
			
		||||
    );
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function parseValueFromCommitMessage($value) {
 | 
			
		||||
    return preg_split('/[\s,]+/', $value, $limit = -1, PREG_SPLIT_NO_EMPTY);
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function readValueFromCommitMessage($value) {
 | 
			
		||||
    $this->setValue($value);
 | 
			
		||||
    return $this;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function renderCommitMessageValue(array $handles) {
 | 
			
		||||
    $value = $this->getValue();
 | 
			
		||||
    if (!$value) {
 | 
			
		||||
      return null;
 | 
			
		||||
    }
 | 
			
		||||
    return implode(', ', $value);
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function shouldAppearInConduitDictionary() {
 | 
			
		||||
    return true;
 | 
			
		||||
  }
 | 
			
		||||
 
 | 
			
		||||
@@ -130,14 +130,6 @@ final class DifferentialRevertPlanField
 | 
			
		||||
    return array($xaction->getNewValue());
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function shouldAppearInCommitMessage() {
 | 
			
		||||
    return true;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function renderCommitMessageValue(array $handles) {
 | 
			
		||||
    return $this->getValue();
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function shouldAppearInConduitDictionary() {
 | 
			
		||||
    return true;
 | 
			
		||||
  }
 | 
			
		||||
 
 | 
			
		||||
@@ -45,7 +45,7 @@ abstract class DifferentialCommitMessageCustomField
 | 
			
		||||
  protected function getCustomFieldOrder($key) {
 | 
			
		||||
    $field_list = PhabricatorCustomField::getObjectFields(
 | 
			
		||||
      new DifferentialRevision(),
 | 
			
		||||
      DifferentialCustomField::ROLE_COMMITMESSAGE);
 | 
			
		||||
      PhabricatorCustomField::ROLE_DEFAULT);
 | 
			
		||||
 | 
			
		||||
    $fields = $field_list->getFields();
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user